Синхронизация на объектах String в Java

Question

У меня есть веб-приложение, в котором я нахожусь в процессе тестирования нагрузки / производительности, особенно в отношении функции, где мы ожидаем, что несколько сотен пользователей будут получать доступ к одной и той же странице и будут обновлять ее каждые 10 секунд. Одна из областей улучшения, которую мы обнаружили с помощью этой функции, заключалась в кэшировании ответов от веб-службы в течение некоторого периода времени, поскольку данные не меняются.

После реализации этого базового кеширования в ходе дальнейшего тестирования я обнаружил, что не думаю, как параллельные потоки могут одновременно получать доступ к кешу. Я обнаружил, что в течение ~ 100 мс около 50 потоков пытались извлечь объект из кэша, обнаружили, что срок его действия истек, нажали веб-службу для извлечения данных и затем поместили объект обратно в кеш.

Исходный код выглядел примерно так:

private SomeData[] getSomeDataByEmail(WebServiceInterface service, String email) {

  final String key = "Data-" + email;
  SomeData[] data = (SomeData[]) StaticCache.get(key);

  if (data == null) {
      data = service.getSomeDataForEmail(email);

      StaticCache.set(key, data, CACHE_TIME);
  }
  else {
      logger.debug("getSomeDataForEmail: using cached object");
  }

  return data;
}

Итак, чтобы убедиться, что только один поток вызывал веб-сервис, когда истек срок действия объекта key, я подумал, что мне нужно синхронизировать операцию получения / установки кэша, и казалось, что использование ключа кэша будет хороший кандидат на объект для синхронизации (таким образом, вызовы этого метода для электронной почты b@b.com не будут блокироваться вызовами методов на a@a.com).

Я обновил метод, чтобы он выглядел так:

private SomeData[] getSomeDataByEmail(WebServiceInterface service, String email) {


  SomeData[] data = null;
  final String key = "Data-" + email;

  synchronized(key) {      
    data =(SomeData[]) StaticCache.get(key);

    if (data == null) {
        data = service.getSomeDataForEmail(email);
        StaticCache.set(key, data, CACHE_TIME);
    }
    else {
      logger.debug("getSomeDataForEmail: using cached object");
    }
  }

  return data;
}

Я также добавил строки регистрации для таких вещей, как «до блока синхронизации», «внутри блока синхронизации», «собирается покинуть блок синхронизации» и «после блока синхронизации», чтобы я мог определить, эффективно ли я синхронизировал get / установить операцию.

Однако, похоже, это не сработало. Мои тестовые журналы имеют вывод вроде:

(вывод журнала - «имя потока», «имя регистратора», «сообщение»)
http-80-Processor253 jsp.view-page - getSomeDataForEmail: собирается войти в блок синхронизации
http-80-Processor253 jsp.view-page - getSomeDataForEmail: внутри блока синхронизации
http-80-Processor253 cache.StaticCache - get: объект с ключом [SomeData-test@test.com] истек
http-80-Processor253 cache.StaticCache - получить: ключ [SomeData-test@test.com] возвращаемое значение [ноль]
http-80-Processor263 jsp.view-page - getSomeDataForEmail: собирается войти в блок синхронизации
http-80-Processor263 jsp.view-page - getSomeDataForEmail: внутри блока синхронизации
http-80-Processor263 cache.StaticCache - get: объект с ключом [SomeData-test@test.com] истек
http-80-Processor263 cache.StaticCache - получить: ключ [SomeData-test@test.com] возвращаемое значение [ноль]
http-80-Processor131 jsp.view-page - getSomeDataForEmail: собирается войти в блок синхронизации
http-80-Processor131 jsp.view-page - getSomeDataForEmail: внутри блока синхронизации
http-80-Processor131 cache.StaticCache - get: объект с ключом [SomeData-test@test.com] истек
http-80-Processor131 cache.StaticCache - получить: ключ [SomeData-test@test.com] возвращаемое значение [ноль]
http-80-Processor104 jsp.view-page - getSomeDataForEmail: внутри блока синхронизации
http-80-Processor104 cache.StaticCache - get: объект с ключом [SomeData-test@test.com] истек
http-80-Processor104 cache.StaticCache - получить: ключ [SomeData-test@test.com] возвращаемое значение [ноль]
http-80-Processor252 jsp.view-page - getSomeDataForEmail: собирается войти в блок синхронизации
http-80-Processor283 jsp.view-page - getSomeDataForEmail: собирается войти в блок синхронизации
http-80-Processor2 jsp.view-page - getSomeDataForEmail: собирается войти в блок синхронизации
http-80-Processor2 jsp.view-page - getSomeDataForEmail: внутри блока синхронизации

Я хотел видеть только один поток за раз, входящий / выходящий из блока синхронизации вокруг операций get / set.

Есть ли проблема с синхронизацией на объектах String? Я думал, что ключ кеша будет хорошим выбором, так как он уникален для операции, и хотя в методе объявлено final String key, я думал, что каждый поток получит ссылку на одного и того же объекта. и, следовательно, будет синхронизация на этом единственном объекте.

Что я здесь не так делаю?

Обновление : после просмотра журналов, кажется, что методы с той же логикой синхронизации, где ключ всегда одинаков, например

final String key = "blah";
...
synchronized(key) { ...

не показывают ту же проблему параллелизма - только один поток за раз входит в блок.

Обновление 2 : Спасибо всем за помощь! Я принял первый ответ о intern() ing Strings, который решил мою первоначальную проблему - когда несколько потоков входили в синхронизированные блоки там, где я думал, что не должны, потому что key имели одинаковое значение.

Как уже отмечали другие, использование intern() для такой цели и синхронизация с этими строками действительно оказываются плохой идеей - когда я запускал тесты JMeter для веб-приложения для имитации ожидаемой нагрузки, я видел использованную кучу размер увеличивается почти до 1 Гб всего за 20 минут.

В настоящее время я использую простое решение - просто синхронизировать весь метод - но я действительно похож на примеры кода, предоставляемые martinprobst и MBCook, но так как у меня есть около 7 подобных getData() методов в этом В настоящее время класс (так как ему требуется около 7 различных частей данных из веб-службы), я не хотел добавлять почти дублирующую логику получения и освобождения блокировок для каждого метода. Но это определенно очень, очень ценная информация для будущего использования. Я думаю, что это, в конечном счете, правильные ответы о том, как лучше сделать такую ​​операцию безопасной для потока, и я бы отдал больше голосов за эти ответы, если бы мог!

Answer 1

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

final String firstkey = "Data-" + email;
final String key = firstkey.intern();

Две строки с одним и тем же значением не обязательно являются одним и тем же объектом.

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

Полагаю, вы знаете, что StaticCache все еще должен быть поточно-ориентированным. Но конкуренция там должна быть крошечной по сравнению с тем, что было бы, если бы вы блокировали кеш, а не только ключ при вызове getSomeDataForEmail.

Ответ на вопрос об обновлении :

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

Редактировать

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

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

Вот альтернатива - она ​​все еще использует единственную блокировку, но мы знаем, что нам все равно понадобится одна из них для кеша, и вы говорили о 50 потоках, а не 5000, так что это может быть не фатально. Я также предполагаю, что узким местом производительности здесь является медленная блокировка ввода-вывода в DoSlowThing (), что, следовательно, принесет огромную пользу от отсутствия сериализации. Если это не узкое место, то:

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

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

Этот код НЕ требует, чтобы StaticCache был синхронизирован или поточно-ориентирован. Это необходимо пересмотреть, если какой-либо другой код (например, запланированная очистка старых данных) когда-либо коснется кеша.

IN_PROGRESS - это фиктивное значение - не совсем чистое, но код простой и экономит, имея две хеш-таблицы. Он не обрабатывает InterruptedException, потому что я не знаю, что ваше приложение хочет сделать в этом случае. Кроме того, если DoSlowThing () постоянно завершается ошибкой для данного ключа, этот код в его нынешнем виде не совсем элегантен, поскольку каждый проходящий поток будет повторять его. Поскольку я не знаю, каковы критерии сбоя и могут ли они быть временными или постоянными, я также не обращаюсь с этим, я просто проверяю, что потоки не блокируются навсегда. На практике вы можете поместить в кеш значение данных, которое указывает «недоступно», возможно, с указанием причины, и время ожидания для повторной попытки.

// do not attempt double-check locking here. I mean it.
synchronized(StaticObject) {
    data = StaticCache.get(key);
    while (data == IN_PROGRESS) {
        // another thread is getting the data
        StaticObject.wait();
        data = StaticCache.get(key);
    }
    if (data == null) {
        // we must get the data
        StaticCache.put(key, IN_PROGRESS, TIME_MAX_VALUE);
    }
}
if (data == null) {
    // we must get the data
    try {
        data = server.DoSlowThing(key);
    } finally {
        synchronized(StaticObject) {
            // WARNING: failure here is fatal, and must be allowed to terminate
            // the app or else waiters will be left forever. Choose a suitable
            // collection type in which replacing the value for a key is guaranteed.
            StaticCache.put(key, data, CURRENT_TIME);
            StaticObject.notifyAll();
        }
    }
}

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

Этот код может быть обобщен для использования с несколькими кешами, если вы определите подходящие абстракции для кеша и связанной с ним блокировки, возвращаемых данных, пустышки IN_PROGRESS и медленной операции для выполнения. Добавление всей информации в метод кеша может быть плохой идеей.

Answer 2

Синхронизация в строке intern'd может быть не очень хорошей идеей - благодаря интернированию String превращается в глобальный объект, и если вы синхронизируете одни и те же строки в разных частях вашего приложения, вы можете получить действительно странные и в основном неразрешимые проблемы синхронизации, такие как взаимоблокировки. Это может показаться маловероятным, но когда это происходит, вы действительно облажались. Как правило, синхронизируйте только локальный объект, когда вы абсолютно уверены, что никакой код вне вашего модуля не сможет его заблокировать.

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

например:.

Object data = StaticCache.get(key, ...);
if (data == null) {
  Object lock = lockTable.get(key);
  if (lock == null) {
    // we're the only one looking for this
    lock = new Object();
    synchronized(lock) {
      lockTable.put(key, lock);
      // get stuff
      lockTable.remove(key);
    }
  } else {
    synchronized(lock) {
      // just to wait for the updater
    }
    data = StaticCache.get(key);
  }
} else {
  // use from cache
}

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

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

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

Answer 3

Строки не хорошие кандидаты для синхронизации. Если вам необходимо выполнить синхронизацию по идентификатору строки, это можно сделать с помощью строки для создания мьютекса (см. « синхронизация по идентификатору »). Стоит ли стоимость этого алгоритма, зависит от того, подразумевает ли вызов вашего сервиса какие-либо существенные операции ввода-вывода.

Также:

• Я надеюсь, что методы StaticCache.get () и set () являются потокобезопасными.
• String.intern () предоставляется по цене (которая варьируется в зависимости от реализации виртуальной машины) и должна использоваться с осторожностью.

Answer 4

Вы можете использовать утилиты параллелизма 1.5 для предоставления кэша, предназначенного для обеспечения множественного одновременного доступа, и единой точки добавления (т. Е. Только один поток когда-либо выполнял дорогостоящее «создание» объекта):

 private ConcurrentMap<String, Future<SomeData[]> cache;
 private SomeData[] getSomeDataByEmail(final WebServiceInterface service, final String email) throws Exception {

  final String key = "Data-" + email;
  Callable<SomeData[]> call = new Callable<SomeData[]>() {
      public SomeData[] call() {
          return service.getSomeDataForEmail(email);
      }
  }
  FutureTask<SomeData[]> ft; ;
  Future<SomeData[]> f = cache.putIfAbsent(key, ft= new FutureTask<SomeData[]>(call)); //atomic
  if (f == null) { //this means that the cache had no mapping for the key
      f = ft;
      ft.run();
  }
  return f.get(); //wait on the result being available if it is being calculated in another thread
}

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

Answer 5

Другие предложили интернировать строки, и это сработает.

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

Я видел два решения этой проблемы:

Вы можете синхронизировать на другом объекте

Вместо электронной почты создайте объект, который содержит электронную почту (скажем, объект «Пользователь»), которая содержит значение электронной почты в качестве переменной. Если у вас уже есть другой объект, представляющий человека (скажем, вы уже что-то извлекли из БД на основе их электронной почты), вы можете использовать его. Реализуя метод equals и метод хэш-кода, вы можете убедиться, что Java считает объекты одинаковыми, когда вы выполняете static cache.contains (), чтобы выяснить, есть ли данные уже в кеше (вам придется синхронизироваться в кеше ).

На самом деле, вы можете оставить вторую Карту, чтобы объекты могли быть заблокированы. Примерно так:

Map<String, Object> emailLocks = new HashMap<String, Object>();

Object lock = null;

synchronized (emailLocks) {
    lock = emailLocks.get(emailAddress);

    if (lock == null) {
        lock = new Object();
        emailLocks.put(emailAddress, lock);
    }
}

synchronized (lock) {
    // See if this email is in the cache
    // If so, serve that
    // If not, generate the data

    // Since each of this person's threads synchronizes on this, they won't run
    // over eachother. Since this lock is only for this person, it won't effect
    // other people. The other synchronized block (on emailLocks) is small enough
    // it shouldn't cause a performance problem.
}

Это предотвратит 15 запросов на один и тот же адрес электронной почты одновременно. Вам нужно что-то, чтобы предотвратить попадание слишком большого количества записей в карту emailLocks. Использование LRUMap s от Apache Commons сделает это.

Это потребует некоторой настройки, но это может решить вашу проблему.

Используйте другую клавишу

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

Краткое описание

Надеюсь, это поможет. Потоки это весело, не правда ли? Вы также можете использовать сеанс, чтобы установить значение, означающее «я уже работаю над поиском этого» и проверить, чтобы увидеть, нужно ли второму (третьему, N-му) потоку попытаться создать или просто подождать, пока результат не появится в кеше. Я думаю, у меня было три предложения.

Answer 6

Вот безопасное короткое решение Java 8, которое использует карту выделенных объектов блокировки для синхронизации:

private static final Map<String, Object> keyLocks = new ConcurrentHashMap<>();

private SomeData[] getSomeDataByEmail(WebServiceInterface service, String email) {
    final String key = "Data-" + email;
    synchronized (keyLocks.computeIfAbsent(key, k -> new Object())) {
        SomeData[] data = StaticCache.get(key);
        if (data == null) {
            data = service.getSomeDataForEmail(email);
            StaticCache.set(key, data);
        }
    }
    return data;
}

У него есть недостаток, заключающийся в том, что ключи и объекты блокировки будут сохраняться на карте вечно.

Это можно обойти примерно так:

private SomeData[] getSomeDataByEmail(WebServiceInterface service, String email) {
    final String key = "Data-" + email;
    synchronized (keyLocks.computeIfAbsent(key, k -> new Object())) {
        try {
            SomeData[] data = StaticCache.get(key);
            if (data == null) {
                data = service.getSomeDataForEmail(email);
                StaticCache.set(key, data);
            }
        } finally {
            keyLocks.remove(key); // vulnerable to race-conditions
        }
    }
    return data;
}

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

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

Так что может быть более безопасно и эффективно использовать срок действия кэша Guava :

private static final LoadingCache<String, Object> keyLocks = CacheBuilder.newBuilder()
        .expireAfterAccess(10, TimeUnit.MINUTES) // max lock time ever expected
        .build(CacheLoader.from(Object::new));

private SomeData[] getSomeDataByEmail(WebServiceInterface service, String email) {
    final String key = "Data-" + email;
    synchronized (keyLocks.getUnchecked(key)) {
        SomeData[] data = StaticCache.get(key);
        if (data == null) {
            data = service.getSomeDataForEmail(email);
            StaticCache.set(key, data);
        }
    }
    return data;
}

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

Answer 7

Используйте подходящую среду кэширования, такую ​​как ehcache .

Реализация хорошего кэша не так проста, как полагают некоторые.

Что касается комментария о том, что String.intern () является источником утечек памяти, то это на самом деле неверно. Interned Strings являются сборщиком мусора, это может занять больше времени, потому что на некоторых JVM'ах (SUN) они хранятся в пермском пространстве, которое затрагивается только полными GC.

Answer 8

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

public class ValueLock<T> {

    private Lock lock = new ReentrantLock();
    private Map<T, Condition> conditions  = new HashMap<T, Condition>();

    public void lock(T t){
        lock.lock();
        try {
            while (conditions.containsKey(t)){
                conditions.get(t).awaitUninterruptibly();
            }
            conditions.put(t, lock.newCondition());
        } finally {
            lock.unlock();
        }
    }

    public void unlock(T t){
        lock.lock();
        try {
            Condition condition = conditions.get(t);
            if (condition == null)
                throw new IllegalStateException();// possibly an attempt to release what wasn't acquired
            conditions.remove(t);
            condition.signalAll();
        } finally {
            lock.unlock();
        }
    }

После (внешней) операции lock (внутренняя) блокировка получается для получения монопольного доступа к карте в течение короткого времени, и, если соответствующий объект уже находится на карте, текущий поток будет ждать, в противном случае он добавит новый Condition на карту, освободит (внутренний) замок и продолжит, и (внешний) замок считается полученным. Операция (внешняя) unlock, сначала получающая (внутреннюю) блокировку, подаст сигнал Condition, а затем удалит объект с карты.

Класс не использует параллельную версию Map, потому что каждый доступ к нему защищен одиночной (внутренней) блокировкой.

Обратите внимание, семантика метода lock() этого класса отличается от семантики ReentrantLock.lock(), повторные вызовы lock() без парных unlock() будут зависать текущий поток бесконечно.

Пример использования, который может быть применим к ситуации, описанный ОП

    ValueLock<String> lock = new ValueLock<String>();
    // ... share the lock   
    String email = "...";
    try {
        lock.lock(email);
        //... 
    } finally {
        lock.unlock(email);
    }

Answer 9

Звонок:

   final String key = "Data-" + email;

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

Это еще раз объяснит вашу правку. Если у вас есть статическая строка, она будет работать.

Использование intern () решает проблему, поскольку возвращает строку из внутреннего пула, хранящегося в классе String, что гарантирует, что если две строки равны, будет использоваться одна из них в пуле. См

http://java.sun.com/j2se/1.4.2/docs/api/java/lang/String.html#intern()

Answer 10

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