Is this the proper way to time concurrent threads in Java

Question

I need to time how long it takes to run each thread of an application I wrote, and I have finished and have a result, but don't really have any good way to verify that I did it right. I've never done anything like this before. If someone could give me a quick proofread it would be very helpful.

Here's the code creating the threads:

for (int i = 0; i < ROWS; i++) {
    threads[threadCount] = new Thread(new TextDistanceThread("Macbeth.txt", "Othello.txt", i, 0));
    threads[threadCount++].start();
    threads[threadCount] = new Thread(new TextDistanceThread("Macbeth.txt", "HuckFinn.txt", i, 1));
    threads[threadCount++].start();
    threads[threadCount] = new Thread(new TextDistanceThread("Macbeth.txt", "TomSawyer.txt", i, 2));
    threads[threadCount++].start();
    threads[threadCount] = new Thread(new TextDistanceThread("Othello.txt", "HuckFinn.txt", i, 3));
    threads[threadCount++].start();
    threads[threadCount] = new Thread(new TextDistanceThread("Othello.txt", "TomSawyer.txt", i, 4));
    threads[threadCount++].start();
    threads[threadCount] = new Thread(new TextDistanceThread("TomSawyer.txt", "HuckFinn.txt", i, 5));
    threads[threadCount++].start();
}

And the code for the thread itself:

public void run() {

    long start = ManagementFactory.getThreadMXBean().getCurrentThreadCpuTime();

//DO SOME STUFF

    long end = ManagementFactory.getThreadMXBean().getCurrentThreadCpuTime();

    Driver.timeResults[0][row][col] = end - start;
    Driver.results[row][col] = difference;
}

score 1 · Answer 1 · edited May 23 '17 at 12:29

1

You either want the per-thread elapsed time or the "real" elapsed time from System.currentTime(); your code gets the per-thread time, which isn't always going to be the same as the actual elapsed time. If that's what you intended, your implementation should work.

An easy way to verify timing behavior is to run a task for a known duration of time. Thread.sleep(), for instance. Try comparing Thread.sleep() to busy-waiting (i.e. while(System.currentTimeMillis() < timeInTheFuture) {}), you'll notice the CPU times will likely be different. Don't expect high precision, but you can still use it to verify your assumptions. If you start up five threads that each work for 30 seconds, do you get ~30 seconds back for each thread? Then it's doing what you expect.

That said, it looks like you're storing your timing information in an array, which isn't a good idea. Arrays are not thread-safe. For your case, it'd probably be easiest to just create a ConcurrentHashMap<String, Long> where the key is the thread name, e.g.

timeResults.put(Thread.currentThread().getName(), end - start);

edited May 23 '17 at 12:29

Community

1
1

answered Sep 23 '15 at 22:30

dimo414

47,227
18
148
244

Good point with the Thread.sleep() idea. As for the array, I thought of that, but is it really a problem if all the threads are guaranteed to have completed before I read the array? For instance, if I use Thread.join()? – Stephen Bendl Sep 23 '15 at 22:51
@Stephen Since the array is created before the threads are started, each thread only writes to one singular location and presumably the array is only read after all threads are joined (otherwise yes obvious problem) that claim is wrong. There is no race condition and the program is perfectly valid. Thread creation and thread joining both establish happens before relationships with preceding code in the involved threads, hence no two threads write to the same location and there are no interleaved reads and writes without synchronization between then. – Voo Sep 24 '15 at 15:48
Prefer to use explicitly thread-safe classes for all inter-thread communication. Besides, a map is cleaner for this use case anyways - you're mapping threads to values. – dimo414 Sep 24 '15 at 16:34
@dimo414 If one doesn't understand the issues at hand using a thread safe class will only mask more grave problems. You might prefer a dictionary here (and then you indeed need a concurrent one - why?) but don't argue that arrays aren't safe to use in that situation because that's wrong. – Voo Sep 25 '15 at 07:37
I agree in this case an array would work, but it's easy for your use case to change such that it doesn't. Using a thread-safe data structure is easy to spot and robust against future changes. Sharing an array across threads is an anti-pattern, even if it's not a problem *in this specific case*. – dimo414 Sep 25 '15 at 14:45

score 0 · Answer 2 · answered Sep 23 '15 at 22:22

0

If you want to measure time spent in each thread by CPU then yes, your code looks correct. Note though that it doesn't measure actual time from thread start to when it completes - for that you would use System.nanoTime().

answered Sep 23 '15 at 22:22

wasyl

3,421
3
27
36

Is this the proper way to time concurrent threads in Java

2 Answers2