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

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

40 голосов
/ 25 сентября 2008

У меня есть веб-приложение, в котором я нахожусь в процессе тестирования нагрузки / производительности, особенно в отношении функции, где мы ожидаем, что несколько сотен пользователей будут получать доступ к одной и той же странице и будут обновлять ее каждые 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 различных частей данных из веб-службы), я не хотел добавлять почти дублирующую логику получения и освобождения блокировок для каждого метода. Но это определенно очень, очень ценная информация для будущего использования. Я думаю, что это, в конечном счете, правильные ответы о том, как лучше сделать такую ​​операцию безопасной для потока, и я бы отдал больше голосов за эти ответы, если бы мог!

Ответы [ 17 ]

1 голос
/ 22 августа 2013

Это довольно поздно, но здесь представлено довольно много неверного кода.

В этом примере: 

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;
}

Синхронизация имеет неверную область. Для статического кеша, который поддерживает API get / put, должна быть как минимум синхронизация операций типа get и getIfAbsentPut для безопасного доступа к кешу. Объем синхронизации будет сам кеш.

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

SynchronizedMap может использоваться вместо явной синхронизации, но необходимо соблюдать осторожность. Если используются неправильные API-интерфейсы (получить и поставить вместо putIfAbsent), то операции не будут иметь необходимой синхронизации, несмотря на использование синхронизированной карты. Обратите внимание на сложности, возникающие при использовании putIfAbsent: либо значение пут должно быть вычислено даже в тех случаях, когда оно не нужно (потому что пут не может знать, нужно ли значение пут до тех пор, пока содержимое кеша не будет исследовано), или требует тщательного использование делегирования (скажем, с использованием Future, которое работает, но в некоторой степени является несоответствием; см. ниже), где положительная стоимость получается по требованию, если необходимо.

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

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

0 голосов
/ 06 марта 2019

Вы можете безопасно использовать String.intern для синхронизации, если можете разумно гарантировать, что строковое значение является уникальным в вашей системе. UUIDS - хороший способ приблизиться к этому. Вы можете связать UUID с вашим фактическим строковым ключом, либо через кеш, карту, либо, возможно, даже сохранить UUID в качестве поля на вашем объектном объекте. 

    @Service   
    public class MySyncService{

      public Map<String, String> lockMap=new HashMap<String, String>();

      public void syncMethod(String email) {

        String lock = lockMap.get(email);
        if(lock==null) {
            lock = UUID.randomUUID().toString();
            lockMap.put(email, lock);
        }   

        synchronized(lock.intern()) {
                //do your sync code here
        }
    }
0 голосов
/ 27 мая 2018

Я добавил небольшой класс блокировки, который может блокировать / синхронизировать любой ключ, включая строки.

См. Реализацию для Java 8, Java 6 и небольшой тест.

Java 8: 

public class DynamicKeyLock<T> implements Lock
{
    private final static ConcurrentHashMap<Object, LockAndCounter> locksMap = new ConcurrentHashMap<>();

    private final T key;

    public DynamicKeyLock(T lockKey)
    {
        this.key = lockKey;
    }

    private static class LockAndCounter
    {
        private final Lock lock = new ReentrantLock();
        private final AtomicInteger counter = new AtomicInteger(0);
    }

    private LockAndCounter getLock()
    {
        return locksMap.compute(key, (key, lockAndCounterInner) ->
        {
            if (lockAndCounterInner == null) {
                lockAndCounterInner = new LockAndCounter();
            }
            lockAndCounterInner.counter.incrementAndGet();
            return lockAndCounterInner;
        });
    }

    private void cleanupLock(LockAndCounter lockAndCounterOuter)
    {
        if (lockAndCounterOuter.counter.decrementAndGet() == 0)
        {
            locksMap.compute(key, (key, lockAndCounterInner) ->
            {
                if (lockAndCounterInner == null || lockAndCounterInner.counter.get() == 0) {
                    return null;
                }
                return lockAndCounterInner;
            });
        }
    }

    @Override
    public void lock()
    {
        LockAndCounter lockAndCounter = getLock();

        lockAndCounter.lock.lock();
    }

    @Override
    public void unlock()
    {
        LockAndCounter lockAndCounter = locksMap.get(key);
        lockAndCounter.lock.unlock();

        cleanupLock(lockAndCounter);
    }


    @Override
    public void lockInterruptibly() throws InterruptedException
    {
        LockAndCounter lockAndCounter = getLock();

        try
        {
            lockAndCounter.lock.lockInterruptibly();
        }
        catch (InterruptedException e)
        {
            cleanupLock(lockAndCounter);
            throw e;
        }
    }

    @Override
    public boolean tryLock()
    {
        LockAndCounter lockAndCounter = getLock();

        boolean acquired = lockAndCounter.lock.tryLock();

        if (!acquired)
        {
            cleanupLock(lockAndCounter);
        }

        return acquired;
    }

    @Override
    public boolean tryLock(long time, TimeUnit unit) throws InterruptedException
    {
        LockAndCounter lockAndCounter = getLock();

        boolean acquired;
        try
        {
            acquired = lockAndCounter.lock.tryLock(time, unit);
        }
        catch (InterruptedException e)
        {
            cleanupLock(lockAndCounter);
            throw e;
        }

        if (!acquired)
        {
            cleanupLock(lockAndCounter);
        }

        return acquired;
    }

    @Override
    public Condition newCondition()
    {
        LockAndCounter lockAndCounter = locksMap.get(key);

        return lockAndCounter.lock.newCondition();
    }
}

Java 6:

открытый класс DynamicKeyLock реализует блокировку { приватная финальная статическая ConcurrentHashMap locksMap = new ConcurrentHashMap (); личный финальный ключ T; 

    public DynamicKeyLock(T lockKey) {
        this.key = lockKey;
    }

    private static class LockAndCounter {
        private final Lock lock = new ReentrantLock();
        private final AtomicInteger counter = new AtomicInteger(0);
    }

    private LockAndCounter getLock()
    {
        while (true) // Try to init lock
        {
            LockAndCounter lockAndCounter = locksMap.get(key);

            if (lockAndCounter == null)
            {
                LockAndCounter newLock = new LockAndCounter();
                lockAndCounter = locksMap.putIfAbsent(key, newLock);

                if (lockAndCounter == null)
                {
                    lockAndCounter = newLock;
                }
            }

            lockAndCounter.counter.incrementAndGet();

            synchronized (lockAndCounter)
            {
                LockAndCounter lastLockAndCounter = locksMap.get(key);
                if (lockAndCounter == lastLockAndCounter)
                {
                    return lockAndCounter;
                }
                // else some other thread beat us to it, thus try again.
            }
        }
    }

    private void cleanupLock(LockAndCounter lockAndCounter)
    {
        if (lockAndCounter.counter.decrementAndGet() == 0)
        {
            synchronized (lockAndCounter)
            {
                if (lockAndCounter.counter.get() == 0)
                {
                    locksMap.remove(key);
                }
            }
        }
    }

    @Override
    public void lock()
    {
        LockAndCounter lockAndCounter = getLock();

        lockAndCounter.lock.lock();
    }

    @Override
    public void unlock()
    {
        LockAndCounter lockAndCounter = locksMap.get(key);
        lockAndCounter.lock.unlock();

        cleanupLock(lockAndCounter);
    }


    @Override
    public void lockInterruptibly() throws InterruptedException
    {
        LockAndCounter lockAndCounter = getLock();

        try
        {
            lockAndCounter.lock.lockInterruptibly();
        }
        catch (InterruptedException e)
        {
            cleanupLock(lockAndCounter);
            throw e;
        }
    }

    @Override
    public boolean tryLock()
    {
        LockAndCounter lockAndCounter = getLock();

        boolean acquired = lockAndCounter.lock.tryLock();

        if (!acquired)
        {
            cleanupLock(lockAndCounter);
        }

        return acquired;
    }

    @Override
    public boolean tryLock(long time, TimeUnit unit) throws InterruptedException
    {
        LockAndCounter lockAndCounter = getLock();

        boolean acquired;
        try
        {
            acquired = lockAndCounter.lock.tryLock(time, unit);
        }
        catch (InterruptedException e)
        {
            cleanupLock(lockAndCounter);
            throw e;
        }

        if (!acquired)
        {
            cleanupLock(lockAndCounter);
        }

        return acquired;
    }

    @Override
    public Condition newCondition()
    {
        LockAndCounter lockAndCounter = locksMap.get(key);

        return lockAndCounter.lock.newCondition();
    }
}

Тест: 

public class DynamicKeyLockTest
{
    @Test
    public void testDifferentKeysDontLock() throws InterruptedException
    {
        DynamicKeyLock<Object> lock = new DynamicKeyLock<>(new Object());
        lock.lock();
        AtomicBoolean anotherThreadWasExecuted = new AtomicBoolean(false);
        try
        {
            new Thread(() ->
            {
                DynamicKeyLock<Object> anotherLock = new DynamicKeyLock<>(new Object());
                anotherLock.lock();
                try
                {
                    anotherThreadWasExecuted.set(true);
                }
                finally
                {
                    anotherLock.unlock();
                }
            }).start();
            Thread.sleep(100);
        }
        finally
        {
            Assert.assertTrue(anotherThreadWasExecuted.get());
            lock.unlock();
        }
    }

    @Test
    public void testSameKeysLock() throws InterruptedException
    {
        Object key = new Object();
        DynamicKeyLock<Object> lock = new DynamicKeyLock<>(key);
        lock.lock();
        AtomicBoolean anotherThreadWasExecuted = new AtomicBoolean(false);
        try
        {
            new Thread(() ->
            {
                DynamicKeyLock<Object> anotherLock = new DynamicKeyLock<>(key);
                anotherLock.lock();
                try
                {
                    anotherThreadWasExecuted.set(true);
                }
                finally
                {
                    anotherLock.unlock();
                }
            }).start();
            Thread.sleep(100);
        }
        finally
        {
            Assert.assertFalse(anotherThreadWasExecuted.get());
            lock.unlock();
        }
    }
}
0 голосов
/ 26 ноября 2017

В случае, если у других есть подобная проблема, следующий код работает, насколько я могу сказать: 

import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Supplier;

public class KeySynchronizer<T> {

    private Map<T, CounterLock> locks = new ConcurrentHashMap<>();

    public <U> U synchronize(T key, Supplier<U> supplier) {
        CounterLock lock = locks.compute(key, (k, v) -> 
                v == null ? new CounterLock() : v.increment());
        synchronized (lock) {
            try {
                return supplier.get();
            } finally {
                if (lock.decrement() == 0) {
                    // Only removes if key still points to the same value,
                    // to avoid issue described below.
                    locks.remove(key, lock);
                }
            }
        }
    }

    private static final class CounterLock {

        private AtomicInteger remaining = new AtomicInteger(1);

        private CounterLock increment() {
            // Returning a new CounterLock object if remaining = 0 to ensure that
            // the lock is not removed in step 5 of the following execution sequence:
            // 1) Thread 1 obtains a new CounterLock object from locks.compute (after evaluating "v == null" to true)
            // 2) Thread 2 evaluates "v == null" to false in locks.compute
            // 3) Thread 1 calls lock.decrement() which sets remaining = 0
            // 4) Thread 2 calls v.increment() in locks.compute
            // 5) Thread 1 calls locks.remove(key, lock)
            return remaining.getAndIncrement() == 0 ? new CounterLock() : this;
        }

        private int decrement() {
            return remaining.decrementAndGet();
        }
    }
}

В случае OP, он будет использоваться следующим образом: 

private KeySynchronizer<String> keySynchronizer = new KeySynchronizer<>();

private SomeData[] getSomeDataByEmail(WebServiceInterface service, String email) {
    String key = "Data-" + email;
    return keySynchronizer.synchronize(key, () -> {
        SomeData[] existing = (SomeData[]) StaticCache.get(key);
        if (existing == null) {
            SomeData[] data = service.getSomeDataForEmail(email);
            StaticCache.set(key, data, CACHE_TIME);
            return data;
        }
        logger.debug("getSomeDataForEmail: using cached object");
        return existing;
    });
}

Если из синхронизированного кода ничего не следует возвращать, метод синхронизации можно записать так: 

public void synchronize(T key, Runnable runnable) {
    CounterLock lock = locks.compute(key, (k, v) -> 
            v == null ? new CounterLock() : v.increment());
    synchronized (lock) {
        try {
            runnable.run();
        } finally {
            if (lock.decrement() == 0) {
                // Only removes if key still points to the same value,
                // to avoid issue described below.
                locks.remove(key, lock);
            }
        }
    }
}
0 голосов
/ 12 октября 2016

другой способ синхронизации на строковом объекте: 

String cacheKey = ...;

    Object obj = cache.get(cacheKey)

    if(obj==null){
    synchronized (Integer.valueOf(Math.abs(cacheKey.hashCode()) % 127)){
          obj = cache.get(cacheKey)
         if(obj==null){
             //some cal obtain obj value,and put into cache
        }
    }
}
0 голосов
/ 25 сентября 2008

Я бы также предложил полностью избавиться от конкатенации строк, если она вам не нужна. 

final String key = "Data-" + email;

Есть ли другие вещи / типы объектов в кеше, которые используют адрес электронной почты, который вам нужен, чтобы эти дополнительные "Данные-" в начале ключа?

если нет, я бы просто сделал это 

final String key = email;

и вы также избегаете создания лишних строк.

0 голосов
/ 25 сентября 2008

Почему бы просто не отобразить статическую html-страницу, которая будет предоставляться пользователю и обновляться каждые x минут?

Страница:
...