Lock free solution while updating the map and reading it

Question

I am trying to measure how much time each thread takes in inserting to database. I have captured all those performance numbers in a ConcurrentHashMap named histogram like how much time each thread is taking in inserting.

Below is the code in which I am measuring how much time each thread is taking and storing it in a ConcurrentHashMap

class Task implements Runnable {

public static ConcurrentHashMap<Long, AtomicLong> histogram = new ConcurrentHashMap<Long, AtomicLong>();

     @Override
     public void run() {

     try {

     long start = System.nanoTime();

     preparedStatement.executeUpdate(); // flush the records.

     long end = System.nanoTime() - start;

     final AtomicLong before = histogram.putIfAbsent(end / 1000000, new AtomicLong(1L));
          if (before != null) {
               before.incrementAndGet();
          }

         }
     }

    }

So my question is whether the way I am trying to measure how much time each thread is taking and storing all those numbers in a ConcurrentHashMap will be thread safe or not?

I think my whole update operation is Atomic. And I just wanted to see if there are any better approach than this if my whole operation is not Atomic. I am looking for mostly lock free solution.

And then after every thread is finished executing there tasks, I am printing this Histogram map from the main method as I have made that map as Static. So this way is right or not?

public class LoadTest {

public static void main(String[] args) {

//executing all the threads using ExecutorService


//And then I am printing out the historgram that got created in Task class    
System.out.println(Task.histogram);

     }

}

Answer 1

Your code is correct; there is also a (more complex) idiom that avoids instantiating AtomicLong each time. However do note that a "naïve" lock-based solution would probably be just as good due to a very low duration of the critical section.