Pergunta

Eu tenho um webapp que eu estou no meio de fazer alguns testes de carga / performance em, particularily em um recurso onde esperamos algumas centenas de usuários para estar acessando a mesma página e bater de atualização a cada 10 segundos nesta página. Uma área de melhoria que encontramos nós poderíamos fazer com esta função foi para armazenar em cache as respostas do serviço de web por algum período de tempo, uma vez que os dados não está mudando.

Depois de implementar esta cache básico, em alguns mais testes eu descobri que eu não considero como threads simultâneos podem acessar o cache ao mesmo tempo. Descobri que em questão de ~ 100ms, cerca de 50 tópicos estavam tentando buscar o objeto a partir do cache, achando que tinha expirado, batendo o serviço web para buscar os dados, e em seguida, colocar o objeto de volta no cache.

O código original parecia algo como isto:

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

Assim, para se certificar de que apenas um segmento foi chamar o serviço web quando o objeto em key expirado, eu pensei que eu precisava para sincronizar a operação get de cache / set, e parecia que usando a chave de cache seria um bom candidato para um objeto para sincronizar on (desta forma, chamadas para este método para email b@b.com não seria bloqueada por chamadas de método para a@a.com).

Eu atualizei o método para se parecer com isto:

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

Eu também acrescentou linhas de registo para coisas como "antes do bloco de sincronização", "bloco dentro de sincronização", "a ponto de bloquear a sincronização licença", e "após o bloqueio de sincronização", para que eu pudesse determinar se eu estava efetivamente sincronizar o get / operação de conjunto.

No entanto, não parece que isso tem funcionado. Meus logs de teste tem uma saída como:

(saída de log é 'threadname' 'logger name' 'mensagem')
http-80-Processor253 jsp.view páginas - getSomeDataForEmail: prestes a entrar sincronização bloco
http-80-Processor253 jsp.view páginas - getSomeDataForEmail: sincronização dentro do bloco
http-80-Processor253 cache.StaticCache - get: objeto na tecla [SomeData-test@test.com] tenha expirado
http-80-Processor253 cache.StaticCache - get: tecla [SomeData-test@test.com] retornando valor [nulo]
http-80-Processor263 jsp.view páginas - getSomeDataForEmail: prestes a entrar sincronização bloco
http-80-Processor263 jsp.view páginas - getSomeDataForEmail: sincronização dentro do bloco
http-80-Processor263 cache.StaticCache - get: objeto na tecla [SomeData-test@test.com] tenha expirado
http-80-Processor263 cache.StaticCache - get: tecla [SomeData-test@test.com] retornando valor [nulo]
http-80-Processor131 jsp.view páginas - getSomeDataForEmail: prestes a entrar sincronização bloco
http-80-Processor131 jsp.view páginas - getSomeDataForEmail: sincronização dentro do bloco
http-80-Processor131 cache.StaticCache - get: objeto na tecla [SomeData-test@test.com] tenha expirado
http-80-Processor131 cache.StaticCache - get: tecla [SomeData-test@test.com] retornando valor [nulo]
http-80-Processor104 jsp.view páginas - getSomeDataForEmail: sincronização dentro do bloco
http-80-Processor104 cache.StaticCache - get: objeto na tecla [SomeData-test@test.com] tenha expirado
http-80-Processor104 cache.StaticCache - get: tecla [SomeData-test@test.com] retornando valor [nulo]
http-80-Processor252 jsp.view páginas - getSomeDataForEmail: prestes a entrar sincronização bloco
http-80-Processor283 jsp.view páginas - getSomeDataForEmail: prestes a entrar sincronização bloco
http-80-Processor2 jsp.view páginas - getSomeDataForEmail: prestes a entrar sincronização bloco
http-80-Processor2 jsp.view páginas - getSomeDataForEmail: Bloco de sincronização dentro

Eu queria ver apenas um thread por entrada / hora de sair do bloco de sincronização em torno dos / operações definidas get.

Existe um problema na sincronização de objetos String? Eu pensei que a chave de cache seria um good escolha, pois é exclusivo para a operação, e mesmo que o final String key é declarada dentro do método, eu estava pensando que cada thread estaria recebendo uma referência ao o mesmo objeto e, portanto, seria a sincronização neste única objeto.

O que estou fazendo de errado aqui?

Atualizar : depois de olhar mais para os logs, parece que métodos com a mesma lógica de sincronização onde a chave é sempre a mesma, como

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

não apresentam o mesmo problema de concorrência -. Apenas um thread por vez está entrando no bloco

Update 2 : Obrigado a todos pela ajuda! Eu aceitei a primeira resposta sobre intern()ing Cordas, que resolveu o meu problema inicial - onde vários segmentos estavam entrando blocos sincronizados em que pensei que não deveria, porque o key teve o mesmo valor

.

Como já foi apontado, usando intern() para tal finalidade e sincronizar nessas cordas, de facto, vir a ser uma má idéia - quando executando testes JMeter contra o webapp para simular a carga esperada, eu vi o tamanho crescer pilha usada a quase 1 GB em apenas 20 minutos.

Atualmente estou usando a solução simples de apenas sincronizar o método inteiro - mas eu realmente como os exemplos de código fornecidos pelo martinprobst e MBCook, mas desde que eu tenho cerca de 7 métodos getData() semelhantes nesta classe atualmente (uma vez que precisa de cerca de 7 peças diferentes de dados de um serviço web), eu não quero adicionar lógica quase duplicado sobre a obtenção e liberando bloqueios para cada método. Mas este é definitivamente informações muito, muito valiosa para uso futuro. Penso que estes são em última análise, as respostas corretas sobre a melhor forma de fazer uma operação como esta thread-safe, e eu dar mais votos a essas respostas se eu pudesse!

Foi útil?

Solução

Sem colocar meu cérebro totalmente em marcha, a partir de uma varredura rápida do que você diz, parece que você precisa estagiário () seu Cordas:

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

duas cordas com o mesmo valor são de outra maneira não necessariamente o mesmo objeto.

Note que isso pode introduzir um novo ponto de discórdia, pois no fundo da VM, estagiário () pode ter que adquirir um bloqueio. Eu não tenho nenhuma idéia do que moderna VMs olhar como nesta área, mas espera-se que eles são diabolicamente otimizado.

Eu suponho que você sabe que StaticCache ainda precisa ser thread-safe. Mas a disputa não deve ser pequena em comparação com o que você teria se estivesse bloqueio em cache em vez de apenas a chave ao chamar getSomeDataForEmail.

Response to update questão :

Eu acho que é porque um string literal sempre produz o mesmo objeto. Dave Costa aponta em um comentário que é ainda melhor do que isso: a literal sempre produz a representação canônica. Assim, todos os literais de string com o mesmo em qualquer valor no programa renderia o mesmo objeto.

Editar

Outros têm apontado que sincronizando em cordas estagiário é realmente uma péssima idéia - em parte porque a criação de cadeias de estágio é permitido levá-los a existir em perpetuidade, e em parte porque se mais de um pouco de qualquer código em suas sincroniza programa em cordas de estágio, você tem dependências entre esses pedaços de código e evitar conflitos ou outros bugs pode ser impossível.

Estratégias para evitar isso armazenando um objeto de bloqueio por string chave estão sendo desenvolvidos em outras respostas como eu tipo.

Aqui está uma alternativa - ele ainda usa um bloqueio singular, mas sabemos que vamos precisar de um daqueles para o cache de qualquer maneira, e você estava falando sobre 50 tópicos, e não 5000, de modo que não pode ser fatal. Eu também estou assumindo que o gargalo de desempenho aqui é lento bloqueando I / O em DoSlowThing () que irá, portanto, extremamente beneficiar não sendo serializado. Se isso não é o gargalo, então:

  • Se a CPU estiver ocupada, então esta abordagem pode não ser suficiente e você precisa de uma outra abordagem.
  • Se a CPU não está ocupado, e acesso ao servidor não é um gargalo, então esta abordagem é um exagero, e você pode também esquecer o mesmo e per-chave de bloqueio, colocar um grande sincronizado (StaticCache) em torno de toda a operação e fazê-lo da maneira mais fácil.

Obviamente, esta abordagem deve ser embeber testada para escalabilidade antes de usar -. Garantia de que nada

Este código não exige que StaticCache é sincronizado ou não thread-safe. Que precisa ser revista se houver outro código (por exemplo programada clean-up dos dados de idade) nunca toca o cache.

IN_PROGRESS é um valor fictício - não exatamente limpo, mas simples do código e que economiza ter dois hashtables. Ele não lida com InterruptedException porque eu não sei o que a sua aplicação quer fazer nesse caso. Além disso, se DoSlowThing () falhar de forma consistente para uma determinada chave este código como está não é exatamente elegante, uma vez que cada fio através vai repetir isso. Desde que eu não sei o que os critérios de falha são, e se eles são susceptíveis de ser temporária ou permanente, eu não lidar com isso também, eu apenas certifique-se tópicos não bloqueie sempre. Na prática, você pode querer colocar um valor de dados no cache que indica 'não disponível', talvez com uma razão, e um tempo limite para quando para tentar novamente.

// 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();
        }
    }
}

Toda vez que nada é adicionado ao cache, os tópicos acordar e verificar o cache (não importa qual chave que está depois), por isso é possível obter um melhor desempenho com algoritmos menos controversas. No entanto, muito do que o trabalho terá lugar durante o seu tempo de CPU ociosa copiosa bloqueio de I / O, por isso não pode ser um problema.

Este código poderia ser ligadas em comum-up para uso com vários caches, se você definir abstrações adequadas para o cache e seu bloqueio associado, os dados que revoltas, o manequim IN_PROGRESS, ea operação lenta para executar. Rolar a coisa toda em um método em cache pode não ser uma má idéia.

Outras dicas

A sincronização em um intern'd Cordas pode não ser uma boa idéia em tudo - por internar-lo, as voltas de corda em um objeto global, e se você sincronizar as mesmas cordas internados em diferentes partes do seu aplicativo, você pode ter problemas de sincronização muito estranho e, basicamente, undebuggable tais como impasses. Pode parecer improvável, mas quando isso acontece você está realmente ferrado. Como regra geral, sempre apenas sincronizar em um objeto local onde você está absolutamente certo de que não fora o código do seu módulo pode bloqueá-lo.

No seu caso, você pode usar um hashtable sincronizado para armazenar objetos de bloqueio para suas chaves.

por exemplo:.

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
}

Este código tem uma condição de corrida, onde dois segmentos podem colocar um objeto na tabela de bloqueio após o outro. Este, porém, não deve ser um problema, porque então você só tem mais uma thread chamando o webservice e atualizar o cache, o que não deve ser um problema.

Se você está invalidar o cache depois de algum tempo, você deve verificar se os dados é nulo novamente depois de recuperá-lo a partir do cache, na fechadura! = Caso nulo.

Como alternativa, e muito mais fácil, você pode fazer todo o método de pesquisa de cache ( "getSomeDataByEmail") sincronizados. Isto significa que os tópicos têm de sincronizar quando eles acessar o cache, o que pode ser um problema de desempenho. Mas, como sempre, tente esta solução simples em primeiro lugar e ver se ele é realmente um problema! Em muitos casos, não deve ser, como você provavelmente gastar muito mais tempo a processar o resultado de sincronização.

Strings são não bons candidatos para sincronização. Se você deve sincronizar em um ID String, isso pode ser feito usando a corda para criar uma exclusão mútua (ver " sincronizando em um ID"). Se o custo desse algoritmo vale a pena depende de se invocar o seu serviço envolve qualquer significativa I / O.

Além disso:

  • Espero que o StaticCache.get () e set () métodos são threadsafe.
  • String.intern () tem um custo (que varia entre as implementações de VM) e deve ser usado com cuidado.

Outros sugeriram internar as cordas, e que vai funcionar.

O problema é que o Java tem de manter cordas internados redor. Foi-me dito que ele faz isso mesmo que você não está segurando uma referência porque o valor deve ser o mesmo as alguém próxima vez usa essa string. Isto significa internar todas as cordas podem começar a comer a memória, que com a carga que você está descrevendo poderia ser um grande problema.

Eu vi duas soluções para isso:

Você pode sincronizar em um outro objeto

Em vez do e-mail, fazer um objeto que mantém o e-mail (dizem que o objeto Usuário) que mantém o valor de e-mail como uma variável. Se você já tem outro objeto que representa a pessoa (dizer-lhe uma coisa já retirado do DB com base no seu e-mail), você pode usar isso. Ao implementar o método e o método hashCode você pode certificar-se de Java considera os objetos a mesma quando você faz uma cache.contains estáticos () para descobrir se os dados já estão no cache é igual a (você vai ter que sincronizar sobre o cache ).

Na verdade, você poderia manter um segundo Mapa de objetos para bloquear. Algo parecido com isto:

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

Isto irá prevenir 15 buscas no mesmo endereço de e-mail em um. Você vai precisar de algo para evitar muitas entradas de acabar no mapa emailLocks. Usando LRUMap s do Apache Commons faria fazê-lo.

Isso precisa de alguns ajustes, mas pode resolver seu problema.

Use uma chave diferente

Se você está disposto a colocar-se com possíveis erros (Eu não sei como isso é importante), você pode usar o código hash da string como a chave. ints não precisa ser internado.

Resumo

Espero que isso ajude. Threading é divertido, não é? Você também pode usar a sessão para definir um valor que significa "Eu já estou trabalhando para encontrar este" e verificar isso para ver se o segundo (terceiro, Nth) necessidades de rosca para tentar criar o ou apenas aguardar o resultado para mostrar-se no cache. Eu acho que eu tinha três sugestões.

Você pode usar os 1,5 utilitários de simultaneidade para fornecer um cache projetado para permitir o acesso simultâneo múltiplas, e um único ponto de adição (ou seja, apenas um segmento cada vez executar o objeto caro "criação"):

 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
}

Obviamente, isso não lidar com exceções como você iria querer, e o cache não têm despejo construído dentro. Talvez você possa usá-lo como uma base para mudar sua classe StaticCache, no entanto.

Use um quadro de cache decente, como ehcache .

A implementação de um cache bom não é tão fácil como algumas pessoas acreditam.

Quanto ao comentário que String.intern () é uma fonte de vazamentos de memória, isso não é realmente verdade. Internadas Cordas são lixo coletado, ele só poderia demorar mais tempo porque em certa JVM (SUN) eles são armazenados no espaço Perm que só é tocada por completo GC.

Aqui é uma solução segura curta Java 8 que usa um mapa de bloqueio dedicado objetos para sincronização:

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

Ele tem uma desvantagem que as chaves e objetos de bloqueio manteria no mapa para sempre.

Isto pode ser contornado como esta:

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

Mas então chaves populares seriam constantemente reinseridas no mapa com bloqueio de objetos que estão sendo realocados.

Atualizar :. E essa condição de corrida folhas possibilidade quando dois threads iria simultaneamente Entre na Seção sincronizados para a mesma chave, mas com diferentes bloqueios

Por isso, pode ser mais seguro e eficiente de usar expirar Goiaba Cache :

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

Note que é assumido aqui que StaticCache é thread-safe e não sofreria de leituras simultâneas e escreve para as chaves diferentes.

Seu principal problema não é apenas que pode haver várias instâncias de String com o mesmo valor. O principal problema é que você precisa ter apenas um monitor no qual a sincronizar para acessar o objeto StaticCache. Caso contrário vários segmentos pode acabar simultaneamente modificando StaticCache (ainda que sob diferentes chaves), que provavelmente não suporta a modificação concorrente.

A chamada:

   final String key = "Data-" + email;

cria um novo objeto de cada vez que o método é chamado. Porque esse objeto é o que você usa para bloqueio e cada chamada para esse método cria um novo objeto, então você não está realmente a sincronização de acesso ao mapa com base na chave.

Esta explicar melhor a sua edição. Quando você tem uma corda estática, então ele vai funcionar.

Usando estagiário () resolve o problema, porque ele retorna a seqüência de uma piscina interna mantida pela classe String, que garante que, se duas seqüências são iguais, a única na piscina será usada. Veja

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

Esta questão parece-me um pouco demasiado ampla e, portanto, instigou igualmente amplo conjunto de respostas. Então, eu vou tentar responder a questão I foram redirecionadas de, infelizmente, que um foi fechado como duplicado.

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

Após a operação lock (exterior) o (interior) bloqueio é adquirido para obter um acesso exclusivo para o mapa por um curto período de tempo, e se o objeto correspondente já está no mapa, o atual segmento vai esperar, caso contrário, ele vai colocar nova Condition ao mapa, liberar o bloqueio (interior) e proceder, e o (exterior) de bloqueio é considerado obtido. O (exterior) unlock operação, em primeiro lugar adquirir um bloqueio (interior), irão sinalizar a Condition e, em seguida, remover o objecto a partir do mapa.

A classe não usa versão concorrente de Map, porque cada acesso a ela é guardada por bloqueio único (interior).

Por favor aviso, a semântica do método lock() desta classe é diferente a de ReentrantLock.lock(), as invocações lock() repetido sem unlock() emparelhado irá travar thread atual indefinidamente.

Um exemplo de uso que possa ser aplicável à situação, o OP descrito

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

Este é um pouco tarde, mas há um monte de código incorreto apresentado aqui.

Neste exemplo:

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

A sincronização tem como escopo incorretamente. Para uma cache estática que suporta um get / API colocar, deve haver pelo menos sincronização em torno das operações get e tipo getIfAbsentPut, para acesso seguro à cache. O escopo de sincronização será o próprio cache.

Se as atualizações devem ser feitas para os elementos de dados a si mesmos, que adiciona uma camada adicional de sincronização, que deve ser sobre os elementos de dados individual.

SynchronizedMap pode ser usado no lugar de sincronização explícita, mas cuidado ainda deve ser observada. Se forem utilizadas as APIs erradas (obter e colocar em vez de putIfAbsent), em seguida, as operações não terá a sincronização necessária, apesar do uso do mapa sincronizado. Observe as complicações introduzidas pelo uso de putIfAbsent: Ou, o valor de venda deve ser computado, mesmo em casos em que não é necessário (porque o posto não pode saber se o valor de venda é necessário até que o conteúdo do cache são examinados), ou requer um cuidado uso de delegação. (digamos, usando Futuro, que funciona, mas é um pouco de uma incompatibilidade; veja abaixo), onde o valor de venda é obtido sob demanda, se necessário

O uso de Futuros é possível, mas parece um pouco estranho, e talvez um pouco de overengineering. A API futuro está em seu núcleo para operações assíncronas, em particular, para as operações que não pode concluir imediatamente. Envolvendo futuro muito provavelmente adiciona uma camada de criação de thread -. Extra de complicações provavelmente desnecessárias

O principal problema do uso de futuro para este tipo de operação é que o futuro inerentemente laços em multi-threading. Uso de futuro, quando um novo tópico não é meios necessários ignorando um monte de máquinas de futuro, tornando-se uma API excessivamente pesado para este uso.

Por que não apenas processar uma página html estática que recebe servido para o usuário e regenerado a cada x minutos?

Eu também sugerem a se livrar da concatenação completo, se você não precisa dele.

final String key = "Data-" + email;

Há outras coisas / tipos de objetos em cache que usar o endereço de e-mail que você precisa que extra "Data-" no início da chave?

Se não, eu tinha acabado de fazer essa

final String key = email;

e você evita tudo o que criação corda extra também.

outra maneira de sincronização no objeto string:

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

Em caso outros têm um problema semelhante, as seguintes obras de código, tanto quanto eu posso dizer:

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

No caso do OP, que seria usado como este:

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

Se nada deve ser devolvido a partir do código sincronizado, o método de sincronização pode ser escrito assim:

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

Eu adicionei uma pequena classe de bloqueio que pode bloquear / sincronizar qualquer chave, incluindo cordas.

Veja a implementação de Java 8, Java 6 e um pequeno teste.

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:

classe pública Bloqueio implementos DynamicKeyLock { estática final privado ConcurrentHashMap locksMap = new ConcurrentHashMap (); tecla T final privado;

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

Test:

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

No seu caso, você poderia usar algo como isto (isso não vazar qualquer memória):

private Synchronizer<String> synchronizer = new Synchronizer();

private SomeData[] getSomeDataByEmail(WebServiceInterface service, String email) {
    String key = "Data-" + email;

    return synchronizer.synchronizeOn(key, () -> {

        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;

    });
}

usá-lo basta adicionar uma dependência:

compile 'com.github.matejtymes:javafixes:1.3.0'

Você pode usar com segurança String.intern para sincronizar se você pode razoavelmente garantir que o valor da cadeia é exclusivo em seu sistema. UUIDs são uma boa maneira de abordar esta questão. Você pode associar um UUID com sua chave cadeia real, quer através de um cache, um mapa, ou talvez até mesmo armazenar o UUID como um campo em seu objeto de entidade.

    @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
        }
    }
Licenciado em: CC-BY-SA com atribuição
Não afiliado a StackOverflow
scroll top