The following code needs reviewing. This is the logic which is used for fetching caches & cache managers. This code would get hit a lot, and concurrency has to be properly handled, while ensuring that it does not kill performance.
Azeez ---------- Forwarded message ---------- From: <[email protected]> Date: Sat, Jul 6, 2013 at 8:03 AM Subject: [Commits] [Carbon-kernel] svn commit r177499 - carbon/kernel/trunk/core/javax.cache/src/main/java/org/wso2/carbon/caching/internal To: [email protected] Author: [email protected] Date: Sat Jul 6 08:03:41 2013 New Revision: 177499 URL: http://wso2.org/svn/browse/wso2?view=rev&revision=177499 Log: Prevent unnecessary synchronization to optimize performance Modified: carbon/kernel/trunk/core/javax.cache/src/main/java/org/wso2/carbon/caching/internal/CacheBuilderImpl.java carbon/kernel/trunk/core/javax.cache/src/main/java/org/wso2/carbon/caching/internal/CacheManagerFactoryImpl.java carbon/kernel/trunk/core/javax.cache/src/main/java/org/wso2/carbon/caching/internal/CarbonCacheManager.java Modified: carbon/kernel/trunk/core/javax.cache/src/main/java/org/wso2/carbon/caching/internal/CacheBuilderImpl.java URL: http://wso2.org/svn/browse/wso2/carbon/kernel/trunk/core/javax.cache/src/main/java/org/wso2/carbon/caching/internal/CacheBuilderImpl.java?rev=177499&r1=177498&r2=177499&view=diff ============================================================================== --- carbon/kernel/trunk/core/javax.cache/src/main/java/org/wso2/carbon/caching/internal/CacheBuilderImpl.java (original) +++ carbon/kernel/trunk/core/javax.cache/src/main/java/org/wso2/carbon/caching/internal/CacheBuilderImpl.java Sat Jul 6 08:03:41 2013 @@ -46,16 +46,14 @@ @Override public Cache<K, V> build() { - synchronized (cacheName.intern()) { - cache = (CacheImpl<K, V>) cacheManager.getExistingCache(cacheName); - if (cache == null) { - cache = new CacheImpl<K, V>(cacheName, cacheManager); - //TODO: set the tenant info - cache.setCacheConfiguration(cacheConfiguration); - cacheManager.addCache(cache); - } + cache = (CacheImpl<K, V>) cacheManager.getExistingCache(cacheName); + if (cache == null) { + cache = new CacheImpl<K, V>(cacheName, cacheManager); + //TODO: set the tenant info + cache.setCacheConfiguration(cacheConfiguration); + cacheManager.addCache(cache); } - return cache; + return (CacheImpl<K, V>) cacheManager.getExistingCache(cacheName); } @Override Modified: carbon/kernel/trunk/core/javax.cache/src/main/java/org/wso2/carbon/caching/internal/CacheManagerFactoryImpl.java URL: http://wso2.org/svn/browse/wso2/carbon/kernel/trunk/core/javax.cache/src/main/java/org/wso2/carbon/caching/internal/CacheManagerFactoryImpl.java?rev=177499&r1=177498&r2=177499&view=diff ============================================================================== --- carbon/kernel/trunk/core/javax.cache/src/main/java/org/wso2/carbon/caching/internal/CacheManagerFactoryImpl.java (original) +++ carbon/kernel/trunk/core/javax.cache/src/main/java/org/wso2/carbon/caching/internal/CacheManagerFactoryImpl.java Sat Jul 6 08:03:41 2013 @@ -67,17 +67,19 @@ @Override public CacheManager getCacheManager(String cacheManagerName) { String tenantDomain = Util.getTenantDomain(); - CacheManager cacheManager; - synchronized ((tenantDomain + "_$_#" + cacheManagerName).intern()) { - Map<String, CacheManager> cacheManagers = globalCacheManagerMap.get(tenantDomain); - if (cacheManagers == null) { - cacheManagers = new ConcurrentHashMap<String, CacheManager>(); - globalCacheManagerMap.put(tenantDomain, cacheManagers); - cacheManager = new CarbonCacheManager(cacheManagerName, this); - cacheManagers.put(cacheManagerName, cacheManager); - } else { - cacheManager = cacheManagers.get(cacheManagerName); - if (cacheManager == null) { + Map<String, CacheManager> cacheManagers = globalCacheManagerMap.get(tenantDomain); + if (cacheManagers == null) { + synchronized (tenantDomain.intern()) { + if ((cacheManagers = globalCacheManagerMap.get(tenantDomain)) == null) { + cacheManagers = new ConcurrentHashMap<String, CacheManager>(); + globalCacheManagerMap.put(tenantDomain, cacheManagers); + } + } + } + CacheManager cacheManager = cacheManagers.get(cacheManagerName); + if (cacheManager == null) { + synchronized ((tenantDomain + "*.*" + cacheManagerName).intern()) { + if ((cacheManager = cacheManagers.get(cacheManagerName)) == null) { cacheManager = new CarbonCacheManager(cacheManagerName, this); cacheManagers.put(cacheManagerName, cacheManager); } Modified: carbon/kernel/trunk/core/javax.cache/src/main/java/org/wso2/carbon/caching/internal/CarbonCacheManager.java URL: http://wso2.org/svn/browse/wso2/carbon/kernel/trunk/core/javax.cache/src/main/java/org/wso2/carbon/caching/internal/CarbonCacheManager.java?rev=177499&r1=177498&r2=177499&view=diff ============================================================================== --- carbon/kernel/trunk/core/javax.cache/src/main/java/org/wso2/carbon/caching/internal/CarbonCacheManager.java (original) +++ carbon/kernel/trunk/core/javax.cache/src/main/java/org/wso2/carbon/caching/internal/CarbonCacheManager.java Sat Jul 6 08:03:41 2013 @@ -108,12 +108,12 @@ if (status != Status.STARTED) { throw new IllegalStateException(); } - Cache<K, V> cache; - synchronized (cacheName.intern()) { - cache = (Cache<K, V>) caches.get(cacheName); - if (cache == null) { - cache = new CacheImpl<K, V>(cacheName, this); - caches.put(cacheName, cache); + Cache<K, V> cache = (Cache<K, V>) caches.get(cacheName); + if (cache == null) { + synchronized (cacheName.intern()) { + if ((cache = (Cache<K, V>) caches.get(cacheName)) == null) { + caches.put(cacheName, cache = new CacheImpl<K, V>(cacheName, this)); + } } } return cache; @@ -121,11 +121,7 @@ @SuppressWarnings("unchecked") final <K, V> Cache<K, V> getExistingCache(String cacheName) { - Cache<K, V> cache; - synchronized (cacheName.intern()) { - cache = (Cache<K, V>) caches.get(cacheName); - } - return cache; + return (Cache<K, V>) caches.get(cacheName); } @Override @@ -207,9 +203,7 @@ Util.checkAccess(ownerTenantDomain, ownerTenantId); checkStatusStarted(); String cacheName = cache.getName(); - synchronized (cacheName.intern()) { - caches.put(cacheName, cache); - } + caches.put(cacheName, cache); } @Override _______________________________________________ Commits mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/commits -- *Afkham Azeez* Director of Architecture; WSO2, Inc.; http://wso2.com Member; Apache Software Foundation; http://www.apache.org/ * <http://www.apache.org/>** email: **[email protected]* <[email protected]>* cell: +94 77 3320919 blog: **http://blog.afkham.org* <http://blog.afkham.org>* twitter: **http://twitter.com/afkham_azeez*<http://twitter.com/afkham_azeez> * linked-in: **http://lk.linkedin.com/in/afkhamazeez* * * *Lean . Enterprise . Middleware*
_______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
