DaveAKing / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

MapMakerInternalMap potential performance improvement? #1634

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
In MapMakerInternalMap line 2074:

/**      
* A counter of the number of reads since the last write, used to drain queues 
on a small      
* fraction of read operations.      
*/     
final AtomicInteger readCount = new AtomicInteger(); 

While profiling my code I have noticed that postReadCleanup spends 70% of its 
exec time incrementing this atomic integer.
Incrementing this atomic integer is about 15% of the get operation.
My measurements were done with a sampling profiler at 20 hz on a 80 core 
cc-numa intel box, 
with 64 threads heavily reading from a guava cache (real code not a benchmark).

It looks to me that this counter's purpose is to drain some queue after a 
arbitrary number of reads, and its overhead seems a bit excessive to me.

Can this counter be made NON atomic, maybe even non volatile ? 

the ensuing race condition might be acceptable by eventually draining the 
queues but not after exactly 64 reads ?

Original issue reported on code.google.com by zolyfar...@yahoo.com on 15 Jan 2014 at 5:32

GoogleCodeExporter commented 9 years ago
A followup, 

since in my use case the cache access was in a contended hotspot (caching the 
result of some expensive reflection), I created a LoadingCache implementation 
which is based on jdk ConcurrentHashMap: 
https://code.google.com/p/spf4j/source/browse/trunk/spf4j-core/src/main/java/org
/spf4j/concurrent/UnboundedLoadingCache.java

This allows me to use a simpler and faster implementation where it makes sense.

This implementation is deprecated by the enhancements done to JDK 1.8 
ConcurrentHashMap. (computeIfAbsent(K key, Function<? super K,? extends V> 
mappingFunction))

I believe it is worth improving the CacheBuilder and provide a more lightweight 
implementation where the parameter constraints allow?

Original comment by zolyfar...@yahoo.com on 24 Mar 2014 at 2:32

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:10

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:17

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:07