Потокобезопасная реализация max

Question

Мне нужно реализовать глобальный объект сбора статистики для веб-сервера. У меня есть Statistics singleton, у которого есть метод addSample(long sample), который впоследствии вызывает updateMax. Это должно быть очевидно потокобезопасным. У меня есть этот метод для обновления максимум всей статистики:

AtomicLong max;

private void updateMax(long sample) {
    while (true) {
        long curMax = max.get();
        if (curMax < sample) {
            boolean result = max.compareAndSet(curMax, sample);
            if (result) break;
        } else {
            break;
        }
    }
}

Правильна ли эта реализация? Я использую java.util.concurrent, потому что считаю, что это будет быстрее, чем простой synchronized. Есть ли другой / лучший способ реализовать это?

Answer 1

Я думаю, что это правильно, но я бы, вероятно, немного переписал его для ясности, и определенно добавлю комментарии:

private void updateMax(long sample) {
    while (true) {
        long curMax = max.get();
        if (curMax >= sample) {
            // Current max is higher, so whatever other threads are
            // doing, our current sample can't change max.
            break;
        }

        // Try updating the max value, but only if it's equal to the
        // one we've just seen. We don't want to overwrite a potentially
        // higher value which has been set since our "get" call.
        boolean setSuccessful = max.compareAndSet(curMax, sample);

        if (setSuccessful) {
            // We managed to update the max value; no other threads
            // got in there first. We're definitely done.
            break;
        }

        // Another thread updated the max value between our get and
        // compareAndSet calls. Our sample can still be higher than the
        // new value though - go round and try again.
    }
}

РЕДАКТИРОВАТЬ: Обычно я бы по крайней мере сначала попробуйте синхронизированную версию, и используйте этот код без блокировки только тогда, когда я обнаружил, что это вызывает проблему.

Answer 2

Начиная с Java 8, LongAccumulator был представлен.Рекомендуется как

. Этот класс обычно предпочтительнее AtomicLong, когда несколько потоков обновляют общее значение, которое используется для таких целей, как сбор статистики, а не для детального управления синхронизацией.В условиях низкой конкуренции за обновление оба класса имеют сходные характеристики.Но в условиях высокой конкуренции ожидаемая пропускная способность этого класса значительно выше за счет более высокого потребления пространства.

Вы можете использовать его следующим образом:

LongAccumulator maxId = new LongAccumulator(Long::max, 0); //replace 0 with desired initial value
maxId.accumulate(newValue); //from each thread

Answer 3

В Java 8 вы можете использовать функциональные интерфейсы и простое лямда-выражение, чтобы решить эту проблему одной строкой и без зацикливания:

private void updateMax(long sample) {
    max.updateAndGet(curMax -> (sample > curMax) ? sample : curMax);
}

В решении используется метод updateAndGet(LongUnaryOperator). Текущее значение содержится в curMax, и с помощью условного оператора выполняется простой тест, заменяющий текущее максимальное значение значением выборки, если значение выборки превышает текущее максимальное значение.

Answer 4

как если бы у вас не было выбора ответов, вот мой:

// while the update appears bigger than the atomic, try to update the atomic.
private void max(AtomicDouble atomicDouble, double update) {
    double expect = atomicDouble.get();
    while (update > expect) {
        atomicDouble.weakCompareAndSet(expect, update);
        expect = atomicDouble.get();
    }
}

это более или менее совпадает с принятым ответом, но не использует break или while(true)который мне лично не нравится.

РЕДАКТИРОВАТЬ: только что обнаружил DoubleAccumulator в Java 8. В документации даже сказано, что это для проблем с краткой статистикой, как у вас:

DoubleAccumulator max = new DoubleAccumulator(Double::max, Double.NEGATIVE_INFINITY);
parallelStream.forEach(max::accumulate);
max.get();

Answer 5

Я считаю, что вы сделали правильно, но это более простая версия, которую я тоже считаю правильной.

private void updateMax(long sample){
      //this takes care of the case where between the comparison and update steps, another thread updates the max

      //For example:
      //if the max value is set to a higher max value than the current value in between the comparison and update step
      //sample will be the higher value from the other thread
      //this means that the sample will now be higher than the current highest (as we just set it to the value passed into this function)
      //on the next iteration of the while loop, we will update max to match the true max value
      //we will then fail the while loop check, and be done with trying to update.
      while(sample > max.get()){
          sample = max.getAndSet(sample);  
      }
}