variable synchronizable sincronizar productores procesos objetos mutua hilos exclusion consumidores acceso java multithreading synchronization thread-safety synchronized

sincronizar - synchronizable java



Sincronización en objetos String en Java (16)

Tengo una aplicación web en la que estoy realizando pruebas de carga / rendimiento, particularmente en una función en la que esperamos que unos pocos cientos de usuarios accedan a la misma página y realicen actualizaciones aproximadamente cada 10 segundos en esta página. Un área de mejora que encontramos que podíamos hacer con esta función era almacenar en caché las respuestas del servicio web durante un período de tiempo, ya que los datos no están cambiando.

Después de implementar este almacenamiento en caché básico, en algunas pruebas adicionales descubrí que no consideraba cómo los subprocesos concurrentes podían acceder al caché al mismo tiempo. Descubrí que en cuestión de ~ 100 ms, aproximadamente 50 hilos intentaban recuperar el objeto de la memoria caché, encontrando que había expirado, presionando el servicio web para recuperar los datos, y luego colocando el objeto nuevamente en la memoria caché.

El código original se veía algo como esto:

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

Entonces, para asegurarme de que solo un hilo llamaba al servicio web cuando expiró el objeto en key , pensé que necesitaba sincronizar la operación de obtención / ajuste de caché, y parecía que usar la clave de caché sería un buen candidato para un objeto para sincronizar en (de esta manera, las llamadas a este método para el correo electrónico [email protected] no serían bloqueadas por llamadas de método a aa.com).

Actualicé el método para que se vea así:

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

También agregué líneas de registro para cosas como "antes del bloque de sincronización", "bloque de sincronización interno", "a punto de salir del bloque de sincronización" y "después del bloque de sincronización", para poder determinar si efectivamente estaba sincronizando la operación get / set.

Sin embargo, no parece que esto haya funcionado. Mis registros de prueba tienen resultados como:

(la salida del registro es ''threadname'' ''logger name'' ''message'')
http-80-Processor253 jsp.view-page - getSomeDataForEmail: a punto de ingresar al bloque de sincronización
http-80-Processor253 jsp.view-page - getSomeDataForEmail: dentro del bloque de sincronización
http-80-Processor253 cache.StaticCache - get: object at key [[email protected]] ha expirado
http-80-Processor253 cache.StaticCache - get: clave [[email protected]] devuelve valor [nulo]
http-80-Processor263 jsp.view-page - getSomeDataForEmail: a punto de ingresar al bloque de sincronización
http-80-Processor263 jsp.view-page - getSomeDataForEmail: dentro del bloque de sincronización
http-80-Processor263 cache.StaticCache - get: object at key [[email protected]] ha expirado
http-80-Processor263 cache.StaticCache - get: clave [[email protected]] devuelve valor [nulo]
http-80-Processor131 jsp.view-page - getSomeDataForEmail: a punto de ingresar al bloque de sincronización
http-80-Processor131 jsp.view-page - getSomeDataForEmail: dentro del bloque de sincronización
http-80-Processor131 cache.StaticCache - get: object at key [[email protected]] ha expirado
http-80-Processor131 cache.StaticCache - get: clave [[email protected]] devuelve valor [nulo]
http-80-Processor104 jsp.view-page - getSomeDataForEmail: dentro del bloque de sincronización
http-80-Processor104 cache.StaticCache - get: object at key [[email protected]] ha expirado
http-80-Processor104 cache.StaticCache - get: clave [[email protected]] devuelve valor [nulo]
http-80-Processor252 jsp.view-page - getSomeDataForEmail: a punto de ingresar al bloque de sincronización
http-80-Processor283 jsp.view-page - getSomeDataForEmail: a punto de ingresar al bloque de sincronización
http-80-Processor2 jsp.view-page - getSomeDataForEmail: a punto de ingresar al bloque de sincronización
http-80-Processor2 jsp.view-page - getSomeDataForEmail: dentro del bloque de sincronización

Quería ver solo un hilo a la vez entrando / saliendo del bloque de sincronización alrededor de las operaciones get / set.

¿Hay algún problema en sincronizar objetos String? Pensé que la clave de caché sería una buena opción ya que es única para la operación, y aunque la final String key se declara dentro del método, estaba pensando que cada hilo estaría obteniendo una referencia al mismo objeto y, por lo tanto, sincronización en este único objeto.

¿Qué estoy haciendo mal aquí?

Actualización : después de mirar más en los registros, parece que los métodos con la misma lógica de sincronización donde la clave es siempre la misma, como

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

no muestre el mismo problema de concurrencia: solo un hilo a la vez está ingresando en el bloque.

Actualización 2 : ¡Gracias a todos por la ayuda! Acepté la primera respuesta sobre intern() ing Strings, que resolvió mi problema inicial, donde múltiples hilos entraban en bloques sincronizados donde yo pensaba que no debían, porque la key tenía el mismo valor.

Como han señalado otros, el uso de intern() para tal fin y la sincronización en esas cadenas realmente resulta ser una mala idea: al ejecutar las pruebas de JMeter contra la aplicación web para simular la carga esperada, vi crecer el tamaño de almacenamiento dinámico usado casi 1GB en poco menos de 20 minutos.

Actualmente estoy usando la simple solución de simplemente sincronizar todo el método, pero realmente me gustan los ejemplos de código proporcionados por martinprobst y MBCook, pero dado que tengo alrededor de 7 métodos getData() similares en esta clase actualmente (ya que se necesitan aproximadamente 7 diferentes datos de un servicio web), no quería agregar lógica casi duplicada para obtener y liberar bloqueos para cada método. Pero esta es definitivamente información muy, muy valiosa para uso futuro. Creo que en última instancia, estas son las respuestas correctas sobre la mejor manera de hacer una operación como este thread-safe, ¡y daría más votos a estas respuestas si pudiera!


¿Por qué no simplemente renderizar una página html estática que se entrega al usuario y se regenera cada x minutos?


Aquí hay una solución corta segura de Java 8 que usa un mapa de objetos de bloqueo dedicados para la sincronización:

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

Tiene el inconveniente de que las teclas y los objetos de bloqueo se mantendrían en el mapa para siempre.

Esto se puede solucionar de esta manera:

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

Pero luego las claves populares se reinsertarían constantemente en el mapa con los objetos de bloqueo reasignados.

Actualización : Y esto deja la posibilidad de condición de carrera cuando dos subprocesos entrarían simultáneamente en la sección sincronizada para la misma clave pero con bloqueos diferentes.

Por lo tanto, puede ser más seguro y eficiente usar la caché de guayaba que vence :

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

Tenga en cuenta que se supone aquí que StaticCache es seguro para subprocesos y no sufrirá lecturas concurrentes ni escrituras para claves diferentes.


En caso de que otros tengan un problema similar, el siguiente código funciona, por lo que puedo decir:

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

En el caso del OP, se usaría así:

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

Si no se devuelve nada del código sincronizado, el método de sincronización se puede escribir así:

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


Esta pregunta me parece demasiado amplia y, por lo tanto, instigó un conjunto igualmente amplio de respuestas. Así que intentaré responder a la pregunta de la que me redireccionaron, desafortunadamente esa ha sido cerrada como duplicada.

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

Después de la operación de lock (externa), se adquiere el bloqueo (interno) para obtener un acceso exclusivo al mapa durante un corto período de tiempo, y si el objeto correspondiente ya está en el mapa, el hilo actual esperará, de lo contrario, pondrá una nueva Condition al mapa, suelte el bloqueo (interno) y proceda, y el bloqueo (externo) se considera obtenido. La operación de unlock (externa), al adquirir primero un bloqueo (interno), indicará la Condition y luego eliminará el objeto del mapa.

La clase no usa la versión simultánea de Map , porque cada acceso a ella está protegido por un bloqueo único (interno).

Tenga en cuenta que el método semántico de lock() de esta clase es diferente al de ReentrantLock.lock() , las invocaciones repetidas de lock() sin el unlock() emparejado unlock() colgarán el hilo actual indefinidamente.

Un ejemplo de uso que podría ser aplicable a la situación, el OP descrito

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


Esto es bastante tarde, pero hay muchos códigos incorrectos presentados aquí.

En este ejemplo:

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

La sincronización tiene un alcance incorrecto. Para una memoria caché estática que admita una API get / put, debe haber al menos sincronización en las operaciones de tipo get y getIfAbsentPut, para un acceso seguro a la memoria caché. El alcance de la sincronización será la memoria caché en sí.

Si las actualizaciones se deben realizar en los elementos de datos, eso agrega una capa adicional de sincronización, que debe estar en los elementos de datos individuales.

SynchronizedMap se puede usar en lugar de la sincronización explícita, pero se debe tener cuidado. Si se utilizan las API incorrectas (get y put en lugar de putIfAbsent), las operaciones no tendrán la sincronización necesaria, a pesar del uso del mapa sincronizado. Observe las complicaciones introducidas por el uso de putIfAbsent: O bien, el valor put debe calcularse incluso en los casos en que no es necesario (porque el put no puede saber si el valor put es necesario hasta que se examinan los contenidos del caché), o requiere un cuidadoso uso de la delegación (digamos, usando Future, que funciona, pero es algo así como un desajuste, ver más abajo), donde el valor de venta se obtiene a pedido, si es necesario.

El uso de futuros es posible, pero parece bastante incómodo, y tal vez un poco excesivo de ingeniería. La Future API está en su núcleo para operaciones asincrónicas, en particular, para operaciones que pueden no completarse inmediatamente. Involving Future muy probablemente agrega una capa de creación de subprocesos, probablemente complicaciones adicionales innecesarias.

El principal problema del uso del Futuro para este tipo de operación es que el Futuro se relaciona intrínsecamente con el multihilo. El uso de Future cuando un nuevo hilo no es necesario significa ignorar una gran parte de la maquinaria de Future, por lo que es una API demasiado pesada para este uso.


La llamada:

final String key = "Data-" + email;

crea un nuevo objeto cada vez que se llama al método. Debido a que ese objeto es lo que usa para bloquear, y cada llamada a este método crea un nuevo objeto, entonces no está realmente sincronizando el acceso al mapa en función de la clave.

Esto explica más tu edición. Cuando tienes una cadena estática, entonces funcionará.

El uso de intern () resuelve el problema, ya que devuelve la cadena de un grupo interno mantenido por la clase String, que garantiza que si dos cadenas son iguales, se usará la que está en el grupo. Ver

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


Las cadenas no son buenos candidatos para la sincronización. Si debe sincronizar en un String ID, puede hacerlo utilizando la cadena para crear un mutex (consulte " sincronización en un ID "). Si el costo de ese algoritmo vale la pena depende de si la invocación de su servicio implica alguna E / S significativa.

También:

  • Espero que los métodos StaticCache.get () y set () sean seguros para el hilo.
  • String.intern() tiene un costo (uno que varía según las implementaciones de VM) y debe usarse con cuidado.

Otros han sugerido internar las cadenas, y eso funcionará.

El problema es que Java tiene que mantener cadenas internas. Me dijeron que hace esto incluso si no tienes una referencia porque el valor debe ser el mismo la próxima vez que alguien use esa cadena. Esto significa que todas las cadenas pueden comenzar a consumir memoria, lo que con la carga que describes podría ser un gran problema.

He visto dos soluciones a esto:

Puedes sincronizar en otro objeto

En lugar del correo electrónico, cree un objeto que contenga el correo electrónico (digamos el objeto Usuario) que tenga el valor del correo electrónico como variable. Si ya tiene otro objeto que represente a la persona (supongamos que ya sacó algo del DB en función de su correo electrónico), podría usarlo. Al implementar el método equals y el método hashcode, puedes asegurarte de que Java considera los objetos iguales cuando haces un cache.contains estático () para averiguar si los datos ya están en el caché (deberás sincronizarlos en el caché) )

En realidad, puede mantener un segundo Mapa para que los objetos se bloqueen. Algo como esto:

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

Esto evitará 15 recuperaciones en la misma dirección de correo electrónico a la vez. Necesitará algo para evitar que demasiadas entradas terminen en el mapa de emailLocks. Usar LRUMap s de Apache Commons lo haría.

Esto necesitará algunos ajustes, pero puede resolver su problema.

Usa una llave diferente

Si está dispuesto a tolerar posibles errores (no sé cuán importante es esto), podría usar el código hash de String como clave. los ints no necesitan ser internados.

Resumen

Espero que esto ayude. Enhebrar es divertido, ¿verdad? También podría usar la sesión para establecer un valor que significa "Ya estoy trabajando para encontrar esto" y verifique si el segundo (tercero, enésimo) subproceso necesita intentar crearlo o simplemente espere a que aparezca el resultado. en el caché Creo que tuve tres sugerencias.


Puede utilizar las utilidades de simultaneidad 1.5 para proporcionar una memoria caché diseñada para permitir el acceso simultáneo múltiple y un único punto de adición (es decir, solo un hilo que realiza la "creación" de objetos costosos):

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, esto no maneja las excepciones como te gustaría, y el caché no tiene desalojo integrado. Sin embargo, tal vez podrías usarlo como base para cambiar tu clase de StaticCache.


Sin poner mi cerebro completamente en marcha, a partir de un escaneo rápido de lo que dices parece que necesitas internar () tus Cuerdas:

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

Dos cadenas con el mismo valor no son necesariamente el mismo objeto.

Tenga en cuenta que esto puede introducir un nuevo punto de contención, ya que en lo profundo de la VM, el interno () puede tener que adquirir un bloqueo. No tengo idea de cómo son las VM modernas en esta área, pero uno espera que estén optimizadas diabólicamente.

Supongo que sabes que StaticCache aún necesita ser seguro para subprocesos. Pero la contención debería ser pequeña comparada con lo que tendrías si estuvieras bloqueando el caché en lugar de solo la clave mientras llamas a getSomeDataForEmail.

Respuesta a la actualización de la pregunta :

Creo que es porque un literal de cadena siempre produce el mismo objeto. Dave Costa señala en un comentario que es incluso mejor que eso: un literal siempre cede la representación canónica. Entonces, todos los literales String con el mismo valor en cualquier parte del programa producirían el mismo objeto.

Editar

Otros han señalado que la sincronización en cadenas internas es realmente una mala idea , en parte porque se permite que la creación de cadenas internas exista a perpetuidad, y en parte porque si más de un bit de código en cualquier parte de su programa se sincroniza en cadenas internas, usted tiene dependencias entre esos bits de código, y la prevención de interbloqueos u otros errores puede ser imposible.

Las estrategias para evitar esto almacenando un objeto de bloqueo por cadena de clave se están desarrollando en otras respuestas a medida que escribo.

Aquí hay una alternativa: todavía usa un bloqueo singular, pero sabemos que vamos a necesitar uno de esos para el caché de todos modos, y usted estaba hablando de 50 hilos, no de 5000, por lo que puede no ser fatal. También estoy asumiendo que el cuello de botella de rendimiento aquí es lento bloqueando I / O en DoSlowThing () que por lo tanto se beneficiará enormemente de no ser serializado. Si ese no es el cuello de botella, entonces:

  • Si la CPU está ocupada, este enfoque puede no ser suficiente y necesita otro enfoque.
  • Si la CPU no está ocupada, y el acceso al servidor no es un cuello de botella, entonces este enfoque es excesivo, y también podría olvidarse de este y del bloqueo por clave, poner una gran sincronización (StaticCache) alrededor de toda la operación, y hacer es el camino fácil.

Obviamente, este enfoque debe ser probado en remojo para su escalabilidad antes de su uso. No garantizo nada.

Este código NO requiere que StaticCache esté sincronizado o que sea seguro para subprocesos. Esto debe revisarse si algún otro código (por ejemplo, limpieza programada de datos antiguos) alguna vez toca la caché.

IN_PROGRESS es un valor ficticio, no exactamente limpio, pero el código es simple y ahorra tener dos hashtables. No maneja InterruptedException porque no sé qué quiere hacer su aplicación en ese caso. Además, si DoSlowThing () falla sistemáticamente para una clave dada, este código, tal como está, no es exactamente elegante, ya que cada subproceso volverá a intentarlo. Dado que no sé cuáles son los criterios de falla, y si son susceptibles de ser temporales o permanentes, tampoco manejo esto, solo me aseguro de que los hilos no bloqueen para siempre. En la práctica, es posible que desee colocar un valor de datos en el caché que indique "no disponible", quizás con un motivo, y un tiempo de espera para cuándo volver a intentarlo.

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

Cada vez que se agrega algo a la caché, todos los subprocesos se activan y verifican la caché (sin importar la clave que busquen), por lo que es posible obtener un mejor rendimiento con menos algoritmos conflictivos. Sin embargo, gran parte de ese trabajo tendrá lugar durante su copioso tiempo de CPU inactivo bloqueando en E / S, por lo que puede no ser un problema.

Este código podría ser común para su uso con cachés múltiples, si define abstracciones adecuadas para la memoria caché y su bloqueo asociado, los datos que devuelve, la variable ficticia IN_PROGRESS y la operación lenta para realizar. Convertir todo en un método en la memoria caché puede no ser una mala idea.


Sincronizar en una cadena interna podría no ser una buena idea, al internarla, la cadena se convierte en un objeto global, y si se sincroniza en las mismas cadenas internas en diferentes partes de la aplicación, puede ser realmente extraño y básicamente problemas de sincronización indebuggable como interbloqueos. Puede parecer poco probable, pero cuando sucede estás realmente jodido. Como regla general, solo sincronice en un objeto local donde esté absolutamente seguro de que ningún código fuera de su módulo podría bloquearlo.

En su caso, puede usar una tabla hash sincronizada para almacenar objetos de bloqueo para sus claves.

P.ej:

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 tiene una condición de carrera, donde dos hilos pueden poner un objeto en la mesa de bloqueo uno después del otro. Sin embargo, esto no debería ser un problema, ya que solo tiene un hilo más que llama al servicio web y actualiza el caché, lo que no debería ser un problema.

Si está invalidando la caché después de un tiempo, debe verificar si los datos vuelven a ser nulos después de recuperarlos de la caché, en el caso de lock! = Null.

Alternativamente, y mucho más fácil, puede hacer que todo el método de búsqueda de caché ("getSomeDataByEmail") se sincronice. Esto significa que todos los hilos tienen que sincronizarse cuando acceden a la memoria caché, lo que podría ser un problema de rendimiento. Pero, como siempre, prueba esta solución simple primero y mira si realmente es un problema. En muchos casos, no debería ser así, ya que probablemente dedique mucho más tiempo procesando el resultado que sincronizando.


Su problema principal no es solo que haya múltiples instancias de String con el mismo valor. El principal problema es que necesita tener solo un monitor en el que sincronizar para acceder al objeto StaticCache. De lo contrario, varios subprocesos podrían terminar modificando StaticCache simultáneamente (aunque con diferentes claves), lo que probablemente no sea compatible con la modificación simultánea.


También te sugiero que te deshagas de la concatenación de cadenas por completo si no la necesitas.

final String key = "Data-" + email;

¿Hay otras cosas / tipos de objetos en el caché que usan la dirección de correo electrónico que necesita ese "Dato-" extra al comienzo de la clave?

si no, solo lo haría

final String key = email;

y evitas toda esa creación de cuerdas extra también.


Use un marco de caché decente como ehcache .

Implementar un buen caché no es tan fácil como algunas personas creen.

En cuanto al comentario de que String.intern () es una fuente de pérdidas de memoria, eso en realidad no es cierto. Las cadenas internas son basura recolectada, simplemente podría llevar más tiempo porque en ciertas JVM''S (SUN) se almacenan en el espacio Perm, que solo se tocan con los GC completos.


de otra manera sincronización en objeto de cadena:

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


He agregado una clase de bloqueo pequeña que puede bloquear / sincronizar en cualquier clave, incluidas las cadenas.

Ver implementación para Java 8, Java 6 y una pequeña prueba.

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:

la clase pública DynamicKeyLock implementa Lock {private final estática ConcurrentHashMap locksMap = new ConcurrentHashMap (); clave T privada final;

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

Prueba:

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