@tagir_valeev16 янв 2015 в 10:09

Ещё раз (надеюсь, последний) про double-checked locking

4 мин

56K

Java * Параллельное программирование *

+43

116

Комментарии 116

@withoutuniverse 16 янв 2015 в 10:19

Какой посыл у вашей статьи?
Код отвратителен, именования переменных тоже.
Как относится prepareFind() с TZDBTimeZoneNames? В статье ни строки про это.
Если TZDB_NAMES_TRIE не константа, зачем в ее имени капс? Почему не sSingleton?

В тоже самое время, в Java отлично работает вот этот код (взято с википедии):

// Работает с новой семантикой volatile
// Не работает в Java 1.4 и более ранних версиях из-за семантики volatile
class Foo {
    private volatile Helper helper = null;
    public Helper getHelper() {
        if (helper == null) {
            synchronized(this) {
                if (helper == null)
                    helper = new Helper();
            }
        }
        return helper;
    }
 
    // и остальные члены класса…
}

@tagir_valeev 16 янв 2015 в 10:24

Вы, наверно, в текст не вчитывались. В статье приведён не идеальный код вида «делайте так», а реальный код (вида «так не делайте») из реальных приложений, которыми пользуются люди. Идеального DCL-кода на Хабре уже было много, пройдите по ссылкам в начале статьи.

@withoutuniverse 16 янв 2015 в 11:55

Так какой же в итоге посыл статьи? «не надо так делать»?
Везде есть разработчики, которые как разработчики так себе, это нормально.

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

К примеру, я уверен, что вызвав метод prepareFind() в 1000 разных потоков я получу одинаковый результат, т.е. DCL реализован нормально, но вот как он связан с классом TZDBTimeZoneNames я не вижу, потому само место «как не нужно делать» отсутствует в коде.

P.S.
Посмотрел другие ваши статьи — видимо, вы часто встречаетесь с такого рода говнокодом и этот пост стал своеобразной точкой кипения.

@withoutuniverse 16 янв 2015 в 12:03

Правда, я не совсем понял, зачем там вообще нужен DCL, ведь метод ничего не возвращает.

@tagir_valeev 16 янв 2015 в 12:03

Да, посыл статьи — не надо так делать :-) Я бы не стал писать, если б так никто не делал, так делают ведь. Если вы всегда пишете код правильно, это замечательно.

Весь контекст по ссылке на grepcode (привожу её ещё раз). Там сразу видно, что этот метод объявлен в этом классе. Место «как не нужно делать» в приведённом коде присутствует. Судя по тому, что минусанули статью пока только вы, другие читатели всё-таки поняли, что я имел в виду.

@withoutuniverse 16 янв 2015 в 12:12

Мы взрослые люди, какие минусы? не вижу смысла ставить ни плюс, ни минус, если честно (и тем более гадить в карму без оснований).
Если честно, не ожидал, что вы примете мой комментарий вот так, «в штыки».

По ссылке весь этот класс сплошной говнокод (и я понимаю, что его писали не вы), его очень сложно даже читать, не то, чтобы вникать в суть его работы.
Все это конечно же мое имхо, но было бы лучше (если уж решили написать статью по этому поводу) разжевать проблему, а не приводить в примеры код и отправлять на ссылку к исходникам.

«Вот есть реализация DCL, реализована правильно, но не нужна, так как сам метод ничего не возвращает, к тому же использование вот тут… неправильное, так как вот тут… возникнет проблема такая-то.»

@withoutuniverse 16 янв 2015 в 12:16

В том же втором вашем примере, идеально описана проблема, причины возникновения и способ ее решения.

@tagir_valeev 16 янв 2015 в 12:20

Окей, окей, перефразирую: «судя по текущему процентному отношению плюсов и минусов за статью…» Так лучше? :-) Если минус ставили не вы, прошу прощения, совпало по времени, значит :-) Карму вашу я не трогал, честно! Никаких штыков, мир-дружба-жвачка!

@dyadyaSerezha 16 янв 2015 в 16:26

… а не приводить в примеры код и отправлять на ссылку к исходникам

Какая ссылка, какие исходники? Приведеннего кода более, чем достаточно, для понимания проблемы и ее решения. Видимо, вы вообще не поняли сути.

@withoutuniverse 16 янв 2015 в 19:06

Ну так ткните меня носом в место в статье, где DCL в первом примере реализован неправильно, пожалуйста. Или хотя бы укажите на строчку кода в примере-тесте, которая ломает работоспособность кода. Также объясните, зачем нужен DCL в методе, который ничего не возвращает.

Я утверждаю, что в 1 примере DCL реализован верно, правда метод должен возвращать хоть что-то, иначе смысл его использования в примере теряется. Само же использование переменной, для которой DCL и нужен (зачем ее использовать напрямую, если для этого DCL и был придуман?), которую код инициализирует должно быть обернуто блоком synchronized(TZDBTimeZoneNames.class) {… use TZDB_NAMES_TRIE here ...}. В коде этого я не вижу, по ссылкам пошел, а там говнокод. Собственно все мои комментарии именно об этом, в той или иной форме.

Я бы очень хотел, чтобы автор или любой другой пользователь подтвердил мою правоту или опроверг, но проще ведь поставить минус и пройти дальше. Считаю, что реализация DCL в 1 примере верна, использования я не вижу в статье.

@SVlad 16 янв 2015 в 21:26

правда метод должен возвращать хоть что-то, иначе смысл его использования в примере теряется

В коде по ссылке значение глобальной переменной TZDB_NAMES_TRIE ипользуется после вызова prepareFind. Это плохой стиль, но работает это, как синглтон.
Достаточно очевидно, что паттерн синглтон в данном случае заинлайнен в метод

(TZDBTimeZoneNames.class) {… use TZDB_NAMES_TRIE here ...}

Зачем заворачивать использование? Смысл как раз в том, что бы был один объект, общий для всех потоков, и к которому потоки имели бы одновременный доступ.

Ну так ткните меня носом в место в статье, где DCL в первом примере реализован неправильно, пожалуйста.

Вот цитата из статьи:

По факту публикация объекта TZDB_NAMES_TRIE состоялась в момент volatile-записи: после этого вызовы prepareFind в других потоках будут сразу выходить без синхронизации. При этом после публикации производится множество дополнительных шагов по инициализации.

То есть, после создания экземпляра TextTrieMap в одном потоке, все остальные потоки при вызове prepareFind увидят его в TZDB_NAMES_TRIE. Однако они могут не увидеть (и не обязаны) всю инициализацию TZDB_NAMES_TRIE, которая происходит после создания объекта.
Дальше в статье написано, что это приводит к ArrayIndexOutOfBoundsException и ConcurrentModificationException.

Инициализацию потоки не видят, так как она происходит после записи в volatile поле. А действия с объектом после записи в volatile не обязаны happens before чтения из той же volatile

@dyadyaSerezha 16 янв 2015 в 22:43

Вот это место: TZDB_NAMES_TRIE = new TextTrieMap(true);
Присваивание TZDB_NAMES_TRIE должно быть последним оператором внутри синхронизированного блока, то есть, после полного завершения всей последовательности инициализации, иначе другие потоки сразу видят ненулевую TZDB_NAMES_TRIE, которая еще не полностью инициализирована, и сразу начинают ее использование. С плачевными последствиями.

@ssova 3 ноя 2017 в 17:05

Спасибо за полезный комментарий! Уже было совсем запутался в этой статье, но вы меня спасли)

@dyadyaSerezha 6 ноя 2017 в 09:17

Да не за что! В смысле, баксов 50 не помешали бы. В)

@Flammar 16 янв 2015 в 16:02

Посыл статьи, похоже, такой:

Исправляются такие вещи очень легко: надо сохранить объект в локальную переменную, с её помощью вызвать все необходимые методы для инициализации, а уже потом записать volatile-поле.

Я это уже встречал при разборе получающегося байт-кода и кода, сгенерированного JIT-компилятором: сначала волатильному полю присваивается указатель на выделенный кусок памяти, а следующим шагом вызывается конструктор. Так что, если делать без посредства локальной переменной, существует такой промежуток времени, в течение которого значение волатильного поля уже заполнено, но объект ещё не инициализирован, что и приводит к некорректной работе.

@Sap_ru 16 янв 2015 в 17:30

Это же явная ошибка JIT-компилятора. В Java же запись/чтение ссылок на объекты всегда атомарно.

@grossws 16 янв 2015 в 21:10

Ссылка будет видна, а внутренние структуры не инициализированы или инициализированы частично. И кому от этого легче?

@Sap_ru 16 янв 2015 в 21:16

Нет. Ссылка должна быть видна только после успешной отработки конструктора.

@SVlad 16 янв 2015 в 21:27

А там инициализация данных не в конструкторе, а в цикле после него.

@Sap_ru 16 янв 2015 в 21:52

Ну, так в данном примере ошибка — никто и не спорит.

@grossws 16 янв 2015 в 21:27

Надо глянуть в jls. В любом случае, первый пример из статьи сломан, т. к. после публикации объекта (volatile write) он модифицируется (т. е. с точки зрения приложения не инициализирован). А happens-before между выходом из syncronized и входом в неё уже не произойдёт (т. к. volotile read вернёт не null).

@leventov 16 янв 2015 в 21:31

В том то и дело что «успешная отработка конструктора» понятие довольно зыбкое. См.
shipilev.net/blog/2014/all-fields-are-final/

@Sap_ru 16 янв 2015 в 22:24

И как это к обсуждаемой теме относится? Там я как понял как раз подтверждается атомарность создания объекта:

C tc = // initialize object + metadata
tc.f = 42; // initialize final field
[LoadStore|StoreStore]
c = tc; // publish

@leventov 16 янв 2015 в 22:29

Это если в объекте есть final поле. А если final поля нет, никаких барьеров не будет.

@Sap_ru 17 янв 2015 в 00:11

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

@leventov 17 янв 2015 в 00:45

Нет, должно быть хотя бы одно final поле. Хотя, возможно, в Java 9 будет для всех объектов: bugs.openjdk.java.net/browse/JDK-8031818

@TimReset 16 янв 2015 в 11:53

Интересные примеры, спасибо! А то много раз слышал, как правильно делать, но практические примеры не правильного использования (и нюансы) не видел :)

@withoutuniverse 16 янв 2015 в 11:56

Делайте, как нужно, и не будет проблем )

@esin 16 янв 2015 в 16:00

Да, примеры «как делать не надо» хорошие. Я бы даже эту фразу сделал жирной :)

Особенность DCL-паттерна в том, что момент публикации объекта — это операция volatile-записи, а не выход из секции синхронизации. Поэтому именно volatile-запись должна производиться после полной инициализации объекта.

@Sap_ru 16 янв 2015 в 17:29

В Java же запись и чтение ссылок атомарно — зачем делать volatile?

@Ivanhoe 16 янв 2015 в 17:40

Для того, чтобы изменение увидели другие потоки, для этого он и предназначен. Без volatile велик риск того, что новое значение застрянет в кэше процессорного ядра.

@Sap_ru 16 янв 2015 в 17:56

А как тогда, по Вашему у объектов, существующих одновременно в нескольких потоках, ссылки подсчитывются? ;) Будет же «застревать в кэше» счётчик ссылок? :)
Кроме того — завершение секции synchronize приведёт к записи переменной, т.к. является барьером синхронизации в Java, разве нет?

@Ivanhoe 16 янв 2015 в 20:18

Кроме того — завершение секции synchronize приведёт к записи переменной, т.к. является барьером синхронизации в Java, разве нет?

Вы меня этим озадачили. Так-то оно так, звучит разумно. Я поисследую вопрос.

@Sap_ru 16 янв 2015 в 21:01

Собственно я исследовал :) Если присвоение непосредственно переменной будет только один раз внутри секции синхронизации, то всё должно работать. Единственный гипотетический случай, когда будут побочные эффекты — если JIT компилятор заинлайнит этот метод внутри цикла и вынесет проверку на null за пределы цикла (ни разу не видел и даже не смог искусственно такое поведение получить — сильно не уверен, что такое возможно). Но на сколько я помню раньше я находил в стандарте ограничения на подобную оптимизацию (давно было — не помню).
Поэтому, как я уже ниже писал, предпочитаю саму переменную объекта не делать volatile, а использовать отдельный volatile int флаг — признак инициализированности. Ибо однажды круто напоролся на то, что при активном использовании объекта, который был volatile производительность приложения упала в сорок (!!!) раз. Из-за того, что при каждом обращении отрабатывал атомарный код ссылки и её инкремента/декремента.
Т.е. я обычно пишу так.

volatile int dclFlag; // int в Java на всех существующих платформах атомарный, а boolean - нет.
MTObject mtObject; // Объект, с которым идёт работа в нескольких потоках.
MTObject2 mtObject2; // Ещё объекты.

void doMultithreaded() {
  if (dclFlag==0) { // Один флаг на несколько объектов.
    synchronized (syncObj) {
      if (dclFlag==0) {      
        // Инициализация.
        mtObject = new MTOject();
        mtObject2 = new MTOject2();
        dclFlag = 1; // На большинстве платформ есть атомарная запись единицы. Например ~0 далеко не все платформы умеют атомарно одной командой писать
      };
    };
   // Код работы с переменными.
   for (int i=0; i!=1000; i++)
      mtObject.doSomething(); // В случае если mtObject volatile, то это превращается в ад.
};

@Sap_ru 16 янв 2015 в 21:07

Кстати, там ад наступал даже не из-за того, что была куча лишних атомарных обращений к переменной объекта (хотя это довольно тяжёлая операция), но из-за того, что чтение volatile-переменной тоже барьер и компилятор и JIT крайне не эффективно компилировали код (была куча лишних записей и чтений переменных).

@Sap_ru 16 янв 2015 в 21:14

Т.е. примерно такой код:

volatile String o1="123";
int counter=0;
void test() {
  for( int i=0; i!=1000; i++ ) {
    if( o2.length()==3 ) // Опаньки! Кроме того, что на каждой итерации будет проверка счётчика ссылок, так volatile чтение ещё и барьер оптимизации!
      counter++; // Вот тут тоже будет неэффективный и неоптимизируемый код. 
  };
}

Как-то так. Извините — пишу по памяти.

@leventov 16 янв 2015 в 21:40

Если вы потом собираетесь mtObject читать вообще в обход doMultithreaded(), то, собственно, нахрена тут DCL? DCL это _оптимизация_ кейса когда вы хотите _очень часто_ вызывать doMultithreaded(), но не хотите платить слишком много за лок. Если вы где-то наверху один раз вызываете doMultithreaded() и потом в цикле работаете с объектами, можно выкинуть весь DCL вместе со флагом и любыми волатайлами, делать все проверки под локом.

volatile на инициализируемой переменной нужен, чтобы избежать ситуации, когда один поток внутри лока записал поле, а другой уже вошел в первую проверку, и увидел непустое поле, чтобы второй прочитал все поля объекта корректно.

Кстати в вашем случае, если 2 объекта, отдельный флаг ничем не лучше, а, точнее, даже хуже, чем сделать только один из инициализируемых объектов volatile, записывать его последним и сравнивать с null при входе в doMultithreaded() именно его.

@grossws 16 янв 2015 в 21:49

От reordering не защитит, если потом они не будут читаться в обратном порядке. Т. к. happens before будет только по одному полю.

@leventov 16 янв 2015 в 21:51

Ничего не понял, что не защитит? Надо в первом условии сравнивать именно volatile ссылку, и если она не null, читать все остальные. И все будет хорошо.

@grossws 16 янв 2015 в 22:20

Object o1;
volatile Object o2;

private void m1() {
  o1 = ...;
  o2 = ...; // volatile write, happens before reading o2, 
  // so writting o1 happens before reading o2 (transitive)
}

private void m2() {
  // m1() in other thread
  o1.toString(); // unsafe, no happens before relation to o1 write
}

private void m3() {
  // m1() in other thread
  o1.toString(); // can be either visible or not
  o2.toString();
}

private void m4() {
  // m1() in other thread
  o2.toString(); // volatile read, o1 initialization is visible on next string
  o1.toString();
}

В функции m3 всё может выглядеть рабочим, а может ломаться. В m4 же всё ок. То, что эта перестановка (m4->m3) вносит гейзенбаг может быть неочевидно для модифицирующего код. Решит какой-нибудь junior поставить два независимых вызова по алфавиту, что в этом такого…

Извините, что некорректно использовал термин reordering.

@leventov 16 янв 2015 в 22:27

Если m1() идет параллельно с m2/3/4, вы обязаны все обращения завернуть в if (o2 != null), иначе тупо будет NPE. вряд ли джуниор осмелится заменить проверку o2 != null на o1 != null.

@grossws 16 янв 2015 в 22:33

Тоже верно, если говорить о lazy инициализации.

Если же объекты инициализированы ранее, а в m1 происходит идемпотентная операция, меняющая состояния o1 и o2, то ошибка проявится. Не вижу, правда ни одной причины не использовать в таком случае обычный лок.

@Sap_ru 16 янв 2015 в 22:27

Если volatile будет сам объект, то при частом к нему обращении будет знатная потеря производительности, как на самом обращении, там и на окружающем коде — второй пример с циклом (там, кстати у меня ошибка — вместо o2 должно быть o1).

@grossws 16 янв 2015 в 22:35

Перед использованием достаточно сохранить ссылку на него в локальную переменную, как здесь уже упоминали, чтобы избежать многократных volatile read.

@Sap_ru 16 янв 2015 в 22:39

Можно ссылку сохранить, но тогда нужно помнить об этом каждый раз. И если забыть, то всё будет работать.

@leventov 16 янв 2015 в 22:42

Так в вашем случае нужно не забыть прочитать флаг, иначе будет не просто медленно, а некорректно. А если вы флаг читать не собираетесь, то вам вообще волатайл не нужен тут.

@Sap_ru 16 янв 2015 в 22:51

Или я чего-то не понимаю. Или одно из двух. В общем случае, при DCL Вы тоже должны о синхронизации помнить, т.к. DCL лишь патерн.
Более того — у меня в примере самый что ни на есть чистый DCL! Double-checked locking! Вы по-моему не понимаете, что DCL — патерн именно синхронизации. А уже на чём он выполняется — на отдельном флаге или на самих данных (если язык программирования позволяет) — уже тонкости (разновидности) реализации этого патерна. И я говорю о том, что реализация патерна с полезными данными в качестве флага может иметь весьма не очевидные побочные эффекты в плане производительности.
А Вы не мне почему-то рассказываете про то, что нужно не забыть флаг проверить… Получается, что если мы делаем DCL, то нужно не забыть сделать DCL…

@leventov 16 янв 2015 в 23:03

Определенная логика есть в флаге, если вы очень не любите выносить поля в локалы. Главная проблема в том, что забытие «бесполезного» флага не означает мгновенный фейл тестов (x86). А если у вас volatile на чем-то полезном, его невозможно забыть, потому что он-то вам, собственно, и нужен. + Меньше чтений из памяти, опять же.

@Sap_ru 16 янв 2015 в 23:34

Именно так. Но зато можно забыть, что с этим полем нельзя активно работать (например в циклах). И тоже никакого фейла тестов не будет. Более того на x86 может даже не быть катастрофической просадки производительности. А на каком-нибудь ARM'е оно не в 40 раз замедлится, а в тысячу. И кто его знает, когда это заметят и заметят ли.

@leventov 16 янв 2015 в 23:35

Если не заметят, то может и хрен с ним?! Корректно же

@Sap_ru 17 янв 2015 в 00:14

Ну, тут уже программисту решать, где оно может работать и на сколько нужно оптимизировать.

@leventov 16 янв 2015 в 22:36

Что мешает перед циклом сохранить ссылку в локальную переменную? Потому что для такого цикла вы в любом случае должны быть уверены, что инициализация уже прошла, иначе это тупо нерабочий код. Подумайте сами: для какой-либо синхронизации, вы в любом случае должны сделать хоть одно volatile чтение чего угодно. Если вы не читаете свой флаг, то значит вам синхронизация в принципе не нужна.

@Sap_ru 16 янв 2015 в 22:45

Ссылку можно сохранять, но тогда нужно об этом помнить и если забыть даже в критичном месте, то на тестах оно никак не проявится (кроме производительности). С флагом забыть нельзя (можно только вообще про синхронизацию забыть, но тогда уж ССЗБ).
Ясен перец, что для синхронизации нужно сделать минимум одно volatile чтения. я говор о том, что применяя в качестве флага DCL полезные данные (а не отдельный флаг) можно легко забыть о том, что обращение к этим данным не просто относительно тяжёлая операция, но ещё и «ломает» оптимизацию окружающего кода.

@leventov 16 янв 2015 в 22:52

С флагом забыть можно, и самое скверное, что на x86 у вас скорее всего все будет и так работать…

@grossws 16 янв 2015 в 22:52

Подумайте сами: для какой-либо синхронизации, вы в любом случае должны сделать хоть одно volatile чтение чего угодно.

Строго говоря не чего угодно, а той же переменной. Если смотреть в новый jls8, 17.4.4-5, то там сказано

A write to a volatile variable v (§8.3.1.4) synchronizes-with all subsequent reads of v by any thread (where «subsequent» is defined according to the synchronization order).

@leventov 16 янв 2015 в 22:56

Ну это я обобщил, «что угодно» флаг/не флаг в смысле. Хотя да, на HotSpot (и на других VM думаю тоже), есть такой эффект, что и впрямь можно читать писать что угодно volatile для синхронизации, чем можно иногда пользоваться, писать в заведомо thread-local volatile, чтобы компилятор поставил барьер, не создавая при этом contention по памяти.

Ну и в текущем JLS то же самое, естественно, строго говоря надо читать писать одно место.

@leventov 16 янв 2015 в 22:57

Т. е. получаем костыль вместо Unsafe.storeFence()

@grossws 16 янв 2015 в 23:04

Не тянуться же лишний раз за s.m.Unsafe ,)

@Sap_ru 16 янв 2015 в 22:38

Что мешает doMultithreaded() вызывать часто? Не говоря уже о том, что может быть несколько разных многопоточных функций работающих с этими переменными и достаточно лишней одной из них быть «тяжёлой», что круто повлиять на производительность.
Я, как бы, когда пример писал, то считал, что очевидно, что раз мы говорим о DCL, то какая-то причина, почему DCL нужен, есть. Ясно же что doMultithreaded лишь одна конкретная функция демонстрирующая проблему с DCL, когда в качестве флага используются непосредственно сами полезные дынные.

@leventov 16 янв 2015 в 22:49

Если у вас в любом случае под каждым doMultithreaded() 1000 итераций, экономия одного лока это ничто, скорее сыграет экономия байткодов на DCL, мож какой метод заинлайнится лишний раз. Если есть еще doMt2(), doMt3() и т. д., где нет такого толстого цикла, если вы в этих других методах не читаете свой флаг, они поломаны, а если читаете, вы делаете 1 volatile чтение + 2 не volatile, хотя могли бы 1 + 1. Флаг просто лишним чтением оказывается, если барьерно можно прочитать что-то полезное.

@Sap_ru 16 янв 2015 в 23:02

У меня может быть ещё 10 методов где используются эти переменные. Если у Вас 10 лёгких методов и 1 тяжёлый, то даже относительно редкий вызов тяжёлого, если он замедлится на два-три порядка может оказать заметное влияние на производительность. Более того, где гарантия что эти лёгкие методы в результате.
И разве я говорил о том, что DCL на полезных данных всегда плохо? Я лишь говорю, что при этом могут быть побочные эффекты в плане производительности.
Например, я видел код, в котором был DCL на полезный данных. Но по мере развития проекта эти данные использовались всё чаще и чаще. В конце концов начались проблемы с производительностью. Расследование показало, что виновато часто используемое volatile-поле. Ведь у Вас DCL может один раз отработать при запуске, например, какого-нибудь worker'а, который потом будет с этими данными активно работать (повторная синхронизация уже не нужна будет). И в этом worker'е volatile переменная запросто может использовать очень часто в любых циклах.
В конце-концов DCL не только для синглтонов используется. Это именно патерн синхронизации и использоваться он может для самых различных целей синхронизации.

@leventov 16 янв 2015 в 23:05

Занятно что я всегда DCL воспринимал как чисто теоретическое упражнение, ни разу в жизни его не писал. Приведите пример реально полезного DCL?

@Sap_ru 16 янв 2015 в 23:15

Нагруженное многопоточное приложение, где DCL используется для передачи команд исполнительным потокам. Если исполнительных потоков много и обмен интенсивный, то синхронизация будет очень тяжёлой операцией. В Java8 для этого новые объекты синхронизации ввели и, в принципе, DCL не нужен, вроде как.
А как вы собираетесь сделать легковесную синхронизацию между несколькими десятками потоков иначе? Либо атомарные счётчки городить, либо DCL. Всё остальное тяжёлое получается.

@leventov 16 янв 2015 в 23:20

То есть есть очередь команд и много потоков выгребают из нее наперегонки? Не очень понимаю при чем тут DCL

@Sap_ru 16 янв 2015 в 23:31

Затрудняюсь ответить :) Но однажды я использовал его именно в таком случае. Второй раз — в качестве флага/общих данных. При многопоточной работе и не такое случается. Особенно если нужно эффективно реализовать какие-нибудь очереди/списки при большом количестве параллельно работающих процессов.

@leventov 16 янв 2015 в 23:34

Для эффективных очередей рекомендую github.com/JCTools/JCTools Например см. реализацию github.com/JCTools/JCTools/blob/master/jctools-core/src/main/java/org/jctools/queues/SpmcArrayQueue.java

@Ivanhoe 17 янв 2015 в 08:00

Приведите пример реально полезного DCL?

В Scala с помощью DCL сделаны lazy-значения ) Т.е., насколько я понимаю, компилятор разворачивает lazy val abc = ... в DCL с битовым флагом.

@leventov 16 янв 2015 в 23:10

Ту же самую проблему можно было решить, просто повыносив volatile чтения в локальные переменные (или перекопируя в не-volatile поле объекта, работа с которым идет в одном потоке). Это было бы чуть-чуть быстрее, чем отдельный флаг. Да, есть возможность забыть про вынесение в новом коде и опять получить проблему. Зато теперь у вас есть возможность забыть про флаг и НЕ получить фейл тестов, зато получить баги гораздо позже на другой платформе/обновлении Java, которая поумнеет и станет что-то переставлять агрессивнее, чем раньше.

@Sap_ru 16 янв 2015 в 23:25

Несомненно. Но у вас же синхронизация может быть в одном-двух местах в коде. В остальных случаях те же данные будут использоваться без синхронизации. Получили некоторое задание для рабочего процесса и работаете. Синхронизация с данными ровно в двух местах — выдача задания и проверка задания, а работа с данными — в сотне. Естественно, это могут быть не только данные, а какие-то флаги, используемые многими исполнительными процессами или ещё что-то подобное. Очереди всякие, опять же — там вообще самые разнообразные варианты могут получаться.

@leventov 16 янв 2015 в 23:32

DCL нужен только когда у вас гонка на публикацию _одного и того же_ задания, что выглядит немного как бред. В противном случае, просто спокойно подготовили задание, положили в AtomicReference, потом рабочий поток из AtomicReference задание достает и работает.

@Sap_ru 16 янв 2015 в 23:42

Это может быть не простая очередь FIFO, а какой-нибудь неблокирующий список. Или это может быть не задание а какие-то текущие общие данные для большого количества потоков. Это может быть какой-то интерфейс обработчика какого-то события для большого количества событий.
И я знаю, как делать очереди и вообще не плохо разбираюсь в многопоточном программировании :) И даже AtomicReferece, как правило не нужен — есть же синхронные очереди. Тем не менее, случаи бывают разные.

@TimReset 16 янв 2015 в 21:40

Интересный способ, должен работать из-за read barrier для volatile. Но судя по stackoverflow.com/questions/10620680/why-volatile-in-java-5-doesnt-synchronize-cached-copies-of-variables-with-main не всегда работает корректно :)
А что бы не было каждый раз обращения к volatile полю в цикле, можно его присвоить не volatile полю и обращаться.

@leventov 16 янв 2015 в 21:48

Ну пугайте людей, это был баг в Java. С таким же успехом можно сказать, что память не всегда работает корректно, потому что раз в миллиард лет можно записать байтик и прочитать другой. Триггеры там может поплавились и неправильно сработали, я не знаю. Если на это закладываться ничего разрабатывать вообще нельзя.

@leventov 16 янв 2015 в 21:55

Не заметил, что цикл сам располагается в doMultithreaded(). Таки да, DCL тут просто не нужен.

@Sap_ru 16 янв 2015 в 22:33

И как это эффективно сделать без DCL?

@leventov 16 янв 2015 в 22:41

MTObject mtObject;
MTObject2 mtObject2;

void doMultithreaded() {
    synchronized (syncObj) {
      if (mtObject==null) {      
        // Инициализация.
       mtObject = new MTOject();
        mtObject2 = new MTOject2();
      }
    }
   // Код работы с переменными.
   for (int i=0; i!=1000; i++)
      m1.doSomething();
};

DCL это типа же «продвинутый» наворот, не стоит забывать что существует «просто» locking! :)

@Sap_ru 16 янв 2015 в 23:06

Ну, раз мы говорим о DCL, то по какой-то причине он у нас уже появился :) Значит кто-то где-то с этими данными часто многопоточно работает на чтение (на самом деле это не единственная возможная причина использования DCL — например, может просто не быть «удобного» объекта синхронизации, если пишется какая-то универсальная библиотека).

@Sap_ru 16 янв 2015 в 23:47

И если этот код часто вызывают 50 процессов, то что случиться? Поляжет этот synchorize и положит производительность. Как раз, если тут больше десятка процессов и/или оно часто вызывается, то не нужно обычную синхронизацию делать.

@Maccimo 17 янв 2015 в 06:12

>> volatile int dclFlag; // int в Java на всех существующих платформах атомарный, а boolean — нет.

Откуда дровишки?

JVMS §2.3.4:

Instead, expressions in the Java programming language that operate on boolean values are compiled to use values of the Java Virtual Machine int data type.

@withoutuniverse 22 янв 2015 в 18:28

// В случае если mtObject volatile, то это превращается в ад.

Если при чтении volatile у вас действительно сильно проседает производительность, реализуйте такой код:

// это глобально видимая ссылка на объект
volatile MTObject mtObject;
...
MTObject threadMTObject = mtObject;
...
for (int i=0; i!=1000; i++)
      threadMTObject.doSomething();

В потоке вы можете единожды прочитать из volatile в локальную переменную, и уже ее использовать как угодно потоку.

@Sap_ru 16 янв 2015 в 21:18

Господа минусующие, не изволите указать на мою ошибку в этом рассуждении?

НЛО прилетело и опубликовало эту надпись здесь

@SVlad 16 янв 2015 в 21:30

Так для взаимодействия двух потоков имеет значение happens before. А для него не достаточно одной синхронизации в пишущем потоке, нужна ещё синхронизация в читающем.
Грубо говоря: да, первый поток сбросит изменения в общую память при выходе из synchronize, но второй поток так и будет читать свой L3 кэш.

@Sap_ru 16 янв 2015 в 23:08

Ну, так я и думал. Правда, на когерентных кэшах работать будет :) Я так никогда не делаю ;)

@leventov 17 янв 2015 в 00:50

Это невозможно по семантике ассемблера, и вообще. Кто-то может читать свой кеш только пока кто-то другой еще НЕ сбросил изменения в общую память. Volatile чтение нужно для запрета переупорядочивания, а не для того, чтобы «прочистить» кеш (это работа процессора).

@leventov 17 янв 2015 в 15:25

Строго говоря, тут я ошибался. Это невозможно по семантике x86 ассемблера, но возможно по семантике некоторых других ассемблеров, на которых кеш действительно надо прочищать «руками» и volatile-чтения генерируют соотв. инструкции. Правда не L3, a L1/L2, потому что это L1 находится в ядре, а L3 это общий для всех ядер интерфейс к памяти, про него вообще можно никогда не думать.

См. habrahabr.ru/post/209128/, первую часть статьи и mechanical-sympathy.blogspot.com/2011/07/memory-barriersfences.html?showComment=1407914816407#c2882832980378499377

@leventov 17 янв 2015 в 00:57

В каком то смысле «кешем» можно считать, собственно, регистры с прочитанными значениями из памяти. Регистры-то никуда не сбрасываются сами, поэтому volatile чтение в каком-то смысле прочищает только их, а на деле просто не сохраняет значение volatile поля в регистре, а читает каждый раз по новой.

@Sap_ru 17 янв 2015 в 12:16

А регистры будут сброшены по закрытию секции synchronize ;)

@leventov 17 янв 2015 в 12:20

Ага, особенно регистры другого ядра.

@Sap_ru 25 янв 2015 в 21:16

А зачем нам тут другое ядро? Будут сброшены все изменения внутри секции.

@grossws 16 янв 2015 в 21:31

Только относительно входа в syncronized на тот же лок. Для остальных happens before не возникает.

@Ivanhoe 20 янв 2015 в 10:24

Кроме того — завершение секции synchronize приведёт к записи переменной, т.к. является барьером синхронизации в Java, разве нет?

Я поизучал и поспрашивал на SO. Кратко: volatile тут таки необходим.

Подробно

Возьмем пример

class Foo {
    private volatile Bar _barInstance;
    public Bar getBar() {
        if (_barInstance == null) {
            synchronized(this) { // or synchronized(someLock)
                if (_barInstance == null) {
                    Bar newInstance = new Bar();
                    // possible additional initialization
                    _barInstance = newInstance;
                }
            }
        }
        return _barInstance;
    }
}

Компилятор/рантайм могут переставить инструкции, связанные с инициализацией объекта. Ссылка _barInstance может быть присвоена раньше, чем выполнятся инструкции конструктора. Другой поток может увидеть, что _barInstance и, соответственно, не брать lock. А значит happens-before на этом блоке синхронизации не случится и появляется риск использования недоинициализированного объекта.

@Ivanhoe 20 янв 2015 в 13:30

Господа хабраюзеры, а за что минусы? Если я в чем-то фактически не прав, то напишите, а то ведь так и помру в неведении.

@grossws 20 янв 2015 в 13:41

Здесь, в комментариях уже несколько раз разжевали почему он необходим. Поэтому дополнительный комментарий про то же — не нужен.

В этом топике у меня складывается впечатление, что почти никто из задававших вопросы не читал ответы на эти же вопросы, заданные другими комментаторами.

@Ivanhoe 20 янв 2015 в 13:47

Да, согласен, виноват :) Слишком поспешил.

@Sap_ru 16 янв 2015 в 17:53

Чего минусуете?
Если переменная для DCL одна, то на черта делать её volatile?
Пример:

DCLVarType dclVar;
DCLVarType getDCLVar() {
  if(dclVAR!=null)
    return dclVar;
  synchronized(syncVar) {
    if(dclVAR!=null)
      return dclVar;
    dclVar = new DCLVarType(); // Тут можно долго и сложно конструировать через промежуточную переменную.
    return dclVar;
  };
}

В каком случае оно может не работать?
Сразу каюсь — я уже Java несколько подзабыл.

@withoutuniverse 16 янв 2015 в 19:12

В Java, если вы не используете ключевое слово volatile, одна и таже переменная (как в вашем примере dclVar) в разных потокаж может быть или null или проинициализирована (именно для вашего примера, в общем случае «иметь разные значения»). Цель DCL в том, чтобы вернуть вам уже проинициализированную только 1 раз переменную.

@Sap_ru 16 янв 2015 в 20:42

Ссылки на объекты в Java, на сколько я знаю, атомарны — нельзя два раза проинициализировать объект.

@withoutuniverse 17 янв 2015 в 16:19

Почитайте про кэширование переменных в разных потоках, а также про значение ключевого слова volatile в java.
Без этого слова по факту потоки могут видеть разные значения одной и той же переменной.

@Sap_ru 25 янв 2015 в 21:18

Т.е., получается, что в Java, можно устроить утечку памяти (либо, двойное освобождение объекта) через многопоточную работу с не-volatile объектом? Нельзя же.

@withoutuniverse 26 янв 2015 в 08:52

Вы неверно трактуете значение ключевого слова volatile.
Опишите ваше понимание того, как будет происходить утечка памяти, а сообщество объяснит вам, где вы ошиблись.

@leventov 16 янв 2015 в 21:43

Сколько можно это обсасывать… читайте все предыдущие статьи по DCL, объясняющие потенциальный баг в этом коде

@Sap_ru 16 янв 2015 в 18:08

Сразу скажу (чтобы не закидали какашками), что лично я, как правило, если использую DCL, то предпочитаю иметь отдельный volatile флаг, т.к. пометка самое переменной как volatile может снизить производительность. Т.к. мало того, что каждый раз будет производиться чтение переменной при частом использовании, так чтение volatile ещё и является барьером в Java и сильно снижает общую оптимизацию программы — т.е. все оптимизации будут ограничены участками между использованием volatile переменной, что бывало, производительность на порядок роняло.

@msd 17 янв 2015 в 23:09

пример кода, пожалуйста?

@stalk 16 янв 2015 в 19:34

Спасибо автору! Обязательно посмотрим, поправим.

@stalk 17 янв 2015 в 00:10

github.com/JetBrains/intellij-community/commit/a2ef8c8039cc21fd8fc2ae16b2bbff5550f04594

@tagir_valeev 17 янв 2015 в 14:47

Спасибо за внимание к моим публикациям, оставайтесь на нашем канале :-)

@yetanothercoder 21 янв 2015 в 08:07

видимо внутри идеи многопоточность не используется так интенсивно, раз у ее разработчиков проскакивают такие ошибки.
Но все равно странно, если используется кодревью, то такое обычно выявляется и не доходит до прода.

@tagir_valeev 21 янв 2015 в 08:15

Да в любом большом проекте такие косяки есть. Вы знаете, что в idea.jar 50000 классов? Вы ревьювить это годами будете, а кастомеры новых фич хотят тем временем.

@grossws 16 янв 2015 в 20:32

Заведите, пожалуйста, тикеты в соответствующих проектах, если их ещё нет.

@tagir_valeev 17 янв 2015 в 05:30

Ну для IDEA, как я понял, публикации на Хабре достаточно, чтобы они обратили внимание :-) В ICU4J, так и быть, завёл, раз уж у меня готовый тест-кейс был. Хотя в этом проекте вообще куча серьёзных багов находится статическим анализом, там можно долго заводить.

Вообще это часто гиблое дело, которое съедает много времени. Мало того, что надо зарегистрироваться в куче разнородных баг-трекеров и разобраться с разной вики-разметкой, так ещё приходится убеждать людей, что баг, найденный статическим анализом, действительно может повлечь реальные проблемы. Некоторые требуют конкретного воспроизведения бага, им недостаточно указания на подозрительное место в коде. Некоторые говорят «это было написано давно человеком, который уже с нами не работает, поэтому мы лучше не будем трогать, а то ещё что-нибудь сломаем». В общем, после нескольких попыток я бросил репортить баги каждый раз. В конце концов, FindBugs бесплатен, скачайте, поставьте и пользуйтесь :-)