CODEWITHSUNDEEP
javamultithreadingtimerthread-safety
john
john

Reputation: 11669

Potential race condition in the timer class?

I have written a timer which will measure the performance of a particular code in any multithreaded application. In the below timer, it will also populate the map with how many calls took x milliseconds. I will use this map as part of my histogram to do further analysis, like what percentage of calls took this much milliseconds and etc.

public static class StopWatch {

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

    public static StopWatch getInstance() {
        return new StopWatch();
    }

    private long m_end = -1;
    private long m_interval = -1;
    private final long m_start;

    private StopWatch() {
        m_start = m_interval = currentTime();
    }

    public long getDuration() {
        long result = 0;

        final long startTime = m_start;
        final long endTime = isStopWatchRunning() ? currentTime() : m_end;

        result = convertNanoToMilliseconds(endTime - startTime);

        boolean done = false;
        while (!done) {
            Long oldValue = histogram.putIfAbsent(result, 1L);
            if (oldValue != null) {
                done = histogram.replace(result, oldValue, oldValue + 1);
            } else {
                done = true;
            }
        }

        return result;
    }

    public long getInterval() {
        long result = 0;

        final long startTime = m_interval;
        final long endTime;

        if (isStopWatchRunning()) {
            endTime = m_interval = currentTime();
        } else {
            endTime = m_end;
        }

        result = convertNanoToMilliseconds(endTime - startTime);

        return result;
    }

    public void stop() {
        if (isStopWatchRunning()) {
            m_end = currentTime();
        }
    }

    private long currentTime() {
        return System.nanoTime();
    }

    private boolean isStopWatchRunning() {
        return (m_end <= 0);
    }

    private long convertNanoToMilliseconds(final long nanoseconds) {
        return nanoseconds / 1000000L;
    }
}

For example, this is the way I will use my above timer class to measure the performance of a particular code in my multithreading application:

StopWatch timer = StopWatch.getInstance();
//... some code here to measure
timer.getDuration();

Now my question is - If you look at the getDuration method, I am also populating my map with the information like how many calls took x milliseconds so that I can use that map later on for further analysis like calculating average, median, 95th and 99th percentile. Does my below code thread safe or is there any race condition?

boolean done = false;
while (!done) {
    Long oldValue = histogram.putIfAbsent(result, 1L);
    if (oldValue != null) {
        done = histogram.replace(result, oldValue, oldValue + 1);
    } else {
        done = true;
    }
}

Between the call to Long oldValue = histogram.putIfAbsent(result, 1L); and done = histogram.replace(result, oldValue, oldValue + 1);, the values in the map could have changed. Thus, oldValue could be stale?

Upvotes: 1

Views: 367

Answers (1)

Michael Deardeuff
Michael Deardeuff

Reputation: 10697

The portion you called out looks correct. Yes sometimes oldValue will be stale, but that's why you're looping. Right?

An alternative approach is to put AtomicLongs into the map. Then you put/get the AtomicLong and increment that.

histogram.putIfAbsent(result, new AtomicLong());
histogram.get(result).incrementAndGet();

In java 8 you may be able to use compute and friends to your advantage (test and see which you like the best):

histogram.computeIfAbsent(result, AtomicLong::new);
histogram.get(result).incrementAndGet();

// or
if (histogram.putIfAbsent(result, new AtomicLong(1)) == null)
   histogram.get(result).incrementAndGet();

// or even
histogram.compute(result, ($, current) -> {
   if (current == null) return new AtomicLong(1);
   current.incrementAndGet();
   return current;
});

Upvotes: 2

Related Questions