Sincronización en objetos String en Java
https://stackoverflow.com/questions/133988
-
02-07-2019 - |
italiano
english
français
española
中国
日本の
العربية
Deutsch
한국어
Português
RussianPregunta
Tengo una aplicación web en la que estoy realizando algunas pruebas de carga / rendimiento, especialmente 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. Una de las áreas de mejora que descubrimos que podríamos realizar con esta función fue 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 hilos concurrentes podían acceder al Caché al mismo tiempo. Descubrí que en unos 100 ms, unos 50 subprocesos intentaban recuperar el objeto de la memoria caché, encontrando que había caducado, atacando el servicio web para recuperar los datos y luego volviendo a colocar el objeto en la memoria caché. p>
El código original tenía este aspecto:
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;
}
Por lo tanto, para asegurarme de que solo un subproceso estaba llamando al servicio web cuando el objeto en key expiró, pensé que necesitaba sincronizar la operación de obtención / configuración de caché, y parecía como usar el la clave de caché sería un buen candidato para que un objeto se sincronice (de esta manera, las llamadas a este método para el correo electrónico b@b.com no se bloquearían con las llamadas de método a a@a.com).
Actualicé el método para tener este aspecto:
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", "dentro del bloque de sincronización", a punto de dejar el bloque de sincronización, y "después del bloque de sincronización", por lo que pude determinar si estaba efectivamente sincronizando la operación get / set.
Sin embargo, no parece que esto haya funcionado. Mis registros de prueba tienen una salida como:
(la salida del registro es 'nombre de subproceso' 'nombre del registrador' 'mensaje')
http-80-Processor253 jsp.view-page - getSomeDataForEmail: a punto de entrar en el bloque de sincronización
http-80-Processor253 jsp.view-page - getSomeDataForEmail: dentro del bloque de sincronización
http-80-Processor253 cache.StaticCache - get: object en la clave [SomeData-test@test.com] ha caducado
http-80-Processor253 cache.StaticCache - get: key [SomeData-test@test.com] devolviendo valor [null]
http-80-Processor263 jsp.view-page - getSomeDataForEmail: a punto de entrar en el bloque de sincronización
http-80-Processor263 jsp.view-page - getSomeDataForEmail: dentro del bloque de sincronización
http-80-Processor263 cache.StaticCache - get: object en la clave [SomeData-test@test.com] ha caducado
http-80-Processor263 cache.StaticCache - get: key [SomeData-test@test.com] devolviendo valor [null]
http-80-Processor131 jsp.view-page - getSomeDataForEmail: a punto de entrar en el bloque de sincronización
http-80-Processor131 jsp.view-page - getSomeDataForEmail: dentro del bloque de sincronización
http-80-Processor131 cache.StaticCache - get: object en la clave [SomeData-test@test.com] ha caducado
http-80-Processor131 cache.StaticCache - get: key [SomeData-test@test.com] devolviendo valor [null]
http-80-Processor104 jsp.view-page - getSomeDataForEmail: dentro del bloque de sincronización
http-80-Processor104 cache.StaticCache - get: object en la clave [SomeData-test@test.com] ha caducado
http-80-Processor104 cache.StaticCache - get: key [SomeData-test@test.com] devolviendo valor [null]
http-80-Processor252 jsp.view-page - getSomeDataForEmail: a punto de entrar en el bloque de sincronización
http-80-Processor283 jsp.view-page - getSomeDataForEmail: a punto de entrar en el bloque de sincronización
http-80-Processor2 jsp.view-page - getSomeDataForEmail: a punto de entrar en el 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 de obtener / configurar.
¿Hay algún problema al sincronizar en 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
¿Qué estoy haciendo mal aquí?
Actualizar : después de seguir analizando los registros, parece que se trata de 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 presentan el mismo problema de concurrencia: solo un hilo a la vez está ingresando al bloque.
Actualización 2 : ¡Gracias a todos por la ayuda! Acepté la primera respuesta sobre intern () ing Strings, que resolvió mi problema inicial, donde varios subprocesos ingresaban en bloques sincronizados donde pensé que no deberían hacerlo, porque la tecla ha tenido el mismo valor.
Como han señalado otros, el uso de intern () para tal propósito y la sincronización en esas cadenas realmente resulta ser una mala idea, cuando se ejecutan pruebas de JMeter contra la aplicación web para simular el esperado carga, vi que el tamaño del montón usado creció a casi 1 GB en menos de 20 minutos.
Actualmente estoy usando la solución simple de solo sincronizar todo el método, pero a realmente me gustan los ejemplos de código proporcionados por martinprobst y MBCook, pero como tengo aproximadamente 7 getData de similares ) actualmente en esta clase (ya que necesita aproximadamente 7 datos diferentes de un servicio web), no quería agregar lógica casi duplicada sobre cómo obtener y liberar bloqueos para cada método. Pero esta es definitivamente una información muy, muy valiosa para uso futuro. Creo que estas son, en última instancia, las respuestas correctas sobre la mejor manera de hacer que una operación como esta sea segura para el hilo, ¡y daría más votos a estas respuestas si pudiera!
Solución
Sin poner mi cerebro completamente en marcha, a partir de un rápido análisis de lo que dices, parece que necesitas internar () tus cadenas:
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 el fondo de la VM, intern () puede tener que adquirir un bloqueo. No tengo idea de cómo se ven las máquinas virtuales modernas en esta área, pero se espera que estén optimizadas diabólicamente.
Supongo que sabes que StaticCache aún necesita ser seguro para subprocesos. Pero la disputa debería ser pequeña en comparación con lo que tendrías si estuvieras bloqueando el caché en lugar de solo la clave al llamar a getSomeDataForEmail.
Respuesta a la actualización de la pregunta :
Creo que eso se debe a que una cadena literal siempre produce el mismo objeto. Dave Costa señala en un comentario que es incluso mejor que eso: un literal siempre produce la representación canónica. Por lo tanto, todos los literales de cadena con el mismo valor en cualquier parte del programa producirían el mismo objeto.
Editar
Otros han señalado que sincronizar en cadenas internas es realmente una mala idea , en parte porque se permite crear cadenas internas para hacer que existan a perpetuidad, y en parte porque si hay más de un bit de el código en cualquier parte de su programa se sincroniza en las cadenas internas, usted tiene dependencias entre esos bits de código, y puede que sea imposible evitar puntos muertos u otros errores.
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: aún usa un bloqueo singular, pero sabemos que vamos a necesitar uno de esos para el caché de todos modos, y estabas hablando de 50 hilos, no de 5000, por lo que puede que no sea fatal. También asumo que el cuello de botella en el rendimiento aquí es el bloqueo lento de E / S 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, es posible que este enfoque no sea suficiente y que necesite 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 es mejor que olvide esto y el bloqueo por tecla, ponga una gran sincronización (StaticCache) alrededor de toda la operación. , y hacerlo de la manera más fácil.
Obviamente, este enfoque debe probarse por inmersión para determinar su escalabilidad antes de usarlo, no garantizo nada.
Este código NO requiere que StaticCache esté sincronizado o sea seguro para subprocesos. Es necesario volver a visitarlo si algún otro código (por ejemplo, la limpieza programada de datos antiguos) alguna vez toca el caché.
IN_PROGRESS es un valor ficticio, no es exactamente limpio, pero el código es simple y ahorra tener dos tablas hash. No maneja InterruptedException porque no sé qué quiere hacer tu aplicación en ese caso. Además, si DoSlowThing () falla constantemente para una clave dada, este código no es exactamente elegante, ya que cada subproceso lo reintentará. Como no sé cuáles son los criterios de falla, y si son susceptibles de ser temporales o permanentes, tampoco lo manejo, solo me aseguro de que los hilos no se bloqueen para siempre. En la práctica, es posible que desee poner un valor de datos en la memoria caché que indique "no disponible", quizás con una razón 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 al caché, todos los subprocesos se activan y verifican el caché (sin importar qué clave busquen), por lo que es posible obtener un mejor rendimiento con algoritmos menos polémicos. Sin embargo, gran parte de ese trabajo se llevará a cabo durante el gran tiempo de inactividad de la CPU en el bloqueo de E / S, por lo que puede no ser un problema.
Este código podría ser compartido para su uso con múltiples cachés, si define abstracciones adecuadas para el caché y su bloqueo asociado, los datos que devuelve, el dummy IN_PROGRESS y la operación lenta para realizar. Enrollar todo en un método en el caché podría no ser una mala idea.
Otros consejos
La sincronización en una Cadena interna puede no ser una buena idea. Al internarla, la Cadena se convierte en un objeto global, y si sincroniza en las mismas cadenas internadas en diferentes partes de su aplicación, podría obtener Problemas de sincronización realmente raros y básicamente no descargables, como puntos muertos. 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 llaves.
Por ejemplo:
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 podrían poner un objeto en la tabla de bloqueo uno tras otro. Sin embargo, esto no debería ser un problema, porque entonces solo tienes 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 memoria caché después de algún tiempo, debe verificar si los datos vuelven a ser nulos después de recuperarlos de la memoria caché, en el bloqueo! = caso nulo.
Alternativamente, y mucho más fácil, puede sincronizar todo el método de búsqueda de caché (" getSomeDataByEmail). Esto significará que todos los subprocesos tienen que sincronizarse cuando acceden al caché, lo que podría ser un problema de rendimiento. Pero como siempre, primero pruebe esta solución simple y vea si realmente es un problema. En muchos casos, no debería ser así, ya que probablemente pasará mucho más tiempo procesando el resultado que sincronizando.
Las cadenas son no buenos candidatos para la sincronización. Si debe sincronizar en una ID de cadena, puede hacerlo usando la cadena para crear una exclusión mutua (vea " sincronizando en una 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 las hebras.
- String.intern () tiene un costo (uno que varía entre las implementaciones de VM) y se debe usar con cuidado.
Otros han sugerido internar las cadenas, y eso funcionará.
El problema es que Java tiene que mantener las cadenas internas alrededor. Me dijeron que lo hace incluso si no tiene una referencia porque el valor debe ser el mismo la próxima vez que alguien use esa cadena. Esto significa que internar todas las cadenas puede comenzar a consumir memoria, lo que con la carga que está describiendo podría ser un gran problema.
He visto dos soluciones a esto:
Puedes sincronizar con otro objeto
En lugar del correo electrónico, cree un objeto que contenga el correo electrónico (digamos el objeto Usuario) que contenga el valor del correo electrónico como una variable. Si ya tiene otro objeto que representa a la persona (digamos que ya extrajo algo de la base de datos en función de su correo electrónico), puede usarlo. Al implementar el método equals y el método hashcode, puede asegurarse de que Java considere los objetos iguales cuando haga un caché estático. Contiene () para averiguar si los datos ya están en el caché (tendrá que sincronizar en el caché ).
En realidad, podría 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 búsquedas en la misma dirección de correo electrónico a la una. Necesitará algo para evitar que demasiadas entradas terminen en el mapa de emailLocks. El uso de LRUMap s de Apache Commons hazlo.
Esto necesitará algunos ajustes, pero puede resolver su problema.
Usar una clave diferente
Si estás dispuesto a tolerar posibles errores (no sé qué tan importante es esto), puedes usar el código hash de la cadena como la clave. Los ints no necesitan ser internados.
Summary
Espero que esto ayude. Roscar es divertido, ¿verdad? También puede usar la sesión para establecer un valor que signifique " ya estoy trabajando para encontrar este " y verifique que para ver si el segundo (tercero, Nth) subproceso debe intentar crear o simplemente esperar a que aparezca el resultado en el caché. Supongo que tenía tres sugerencias.
Puede usar las utilidades de concurrencia 1.5 para proporcionar un caché diseñado para permitir el acceso simultáneo múltiple y un único punto de adición (es decir, solo un hilo que realiza el objeto costoso " creación "):
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 controla las excepciones como querrías, y la memoria caché no tiene el desalojo incorporado. Sin embargo, quizás puedas usarlo como base para cambiar tu clase StaticCache.
Use un marco de almacenamiento en 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 fugas de memoria, en realidad no es cierto. Las cadenas internas son recolectadas, es posible que tarde más tiempo porque en ciertas JVM'S (SUN) se almacenan en un espacio de Perm que solo es tocado por GC's completos.
Aquí hay una solución Java 8 corta y segura que utiliza 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 un inconveniente que las claves y los objetos de bloqueo se mantendrían en el mapa para siempre.
Esto se puede solucionar de la siguiente 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); // vulnerable to race-conditions
}
}
return data;
}
Pero entonces las claves populares se reinsertarían constantemente en el mapa con los objetos de bloqueo que se reasignan.
Actualizar : Y esto deja una posibilidad de carrera cuando dos subprocesos entrarán simultáneamente en la sección sincronizada para la misma clave pero con diferentes bloqueos.
Por lo tanto, puede ser más seguro y eficiente usar caducando el caché de guayaba :
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 aquí se supone que StaticCache es seguro para subprocesos y no sufriría lecturas y escrituras simultáneas para diferentes claves.
Su principal problema no es solo que puede haber varias instancias de String con el mismo valor. El problema principal es que necesita tener un solo monitor en el que sincronizar para acceder al objeto StaticCache. De lo contrario, varios subprocesos podrían terminar modificando simultáneamente StaticCache (aunque con diferentes claves), que probablemente no admita la modificación concurrente.
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 usas para bloquear, y cada llamada a este método crea un nuevo objeto, entonces realmente no estás sincronizando el acceso al mapa basado en la clave.
Esto explica con más detalle 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 conservado por la clase String, que asegura que si dos cadenas son iguales, se usará la del grupo. Ver
http: / /java.sun.com/j2se/1.4.2/docs/api/java/lang/String.html#intern ()
Esta pregunta me parece un poco demasiado amplia, y por lo tanto instigó un conjunto de respuestas igualmente amplio. Así que intentaré responder la pregunta He sido redirigido desde, desafortunadamente, uno ha sido cerrado 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();
}
}
Tras la operación (externa) lock , se adquiere la cerradura (interna) para obtener un acceso exclusivo al mapa por un corto tiempo, y si el objeto correspondiente ya está en el mapa, el actual el hilo esperará,
de lo contrario, pondrá una nueva Condición en el mapa, liberará el bloqueo (interno) y procederá,
y la cerradura (exterior) se considera obtenida.
La operación (externa) unlock , que primero adquiere un bloqueo (interno), indicará el Condición y luego eliminará el objeto del mapa.
La clase no usa una versión concurrente de Mapa , porque cada acceso a ella está protegido por un solo bloqueo (interno).
Tenga en cuenta que el método semántico de lock () de esta clase es diferente al de ReentrantLock.lock () , el repetido lock () invocaciones sin unlock () emparejado colgará 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 bastante código incorrecto presentado 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 de obtención / colocación, debe haber al menos sincronización alrededor de las operaciones de tipo get y getIfAbsentPut, para un acceso seguro a la memoria caché. El alcance de la sincronización será el propio caché.
Si se deben realizar actualizaciones a los elementos de datos, eso agrega una capa adicional de sincronización, que debe estar en los elementos de datos individuales.
Se puede utilizar SynchronizedMap en lugar de la sincronización explícita, pero se debe tener cuidado. Si se utilizan las API incorrectas (obtener y poner 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 put no puede saber si el valor put es necesario hasta que se examinen los contenidos del caché), o requiere un cuidado. uso de la delegación (por ejemplo, usar Futuro, que funciona, pero es un tanto un desajuste; consulte a continuación), donde el valor de venta se obtiene a demanda si es necesario.
El uso de Futuros es posible, pero parece bastante incómodo, y quizás un poco de ingeniería excesiva. La API del futuro está en su núcleo para las operaciones asíncronas, en particular para las operaciones que pueden no completarse de inmediato. Involving Future probablemente agrega una capa de creación de subprocesos, probablemente complicaciones adicionales innecesarias.
El principal problema del uso de Futuro para este tipo de operación es que el Futuro se vincula de forma inherente en los subprocesos múltiples. El uso de Future cuando no es necesario un nuevo hilo significa ignorar gran parte de la maquinaria de Future, lo que lo convierte en una API demasiado pesada para este uso.
¿Por qué no solo generar una página html estática que se sirve al usuario y se regenera cada x minutos?
También sugiero deshacerse completamente de la concatenación de cadenas 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 que extra " Datos- " al comienzo de la clave?
si no, simplemente lo haría
final String key = email;
y también evitas toda la creación de cadenas adicionales.
otra forma de sincronización en el 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
}
}
}
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 debe devolver 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);
}
}
}
}
He agregado una pequeña clase de bloqueo que puede bloquear / sincronizar en cualquier clave, incluidas las cadenas.
Consulte la 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:
clase pública DynamicKeyLock implementa Lock { Private estática final ConcurrentHashMap locksMap = new ConcurrentHashMap (); tecla T final privada;
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();
}
}
}
En su caso, podría usar algo como esto (esto no pierde ninguna memoria):
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;
});
}
para usarlo solo agregas una dependencia:
compile 'com.github.matejtymes:javafixes:1.3.0'
Puede usar String.intern de forma segura para sincronizar si puede garantizar razonablemente que el valor de la cadena es único en su sistema. Los UUIDS son una buena manera de abordar esto. Puede asociar un UUID con su clave de cadena real, ya sea a través de un caché, un mapa o tal vez incluso almacenar el uuid como un campo en su objeto de entidad.
@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
}
}
