search

dnrajugade / guava-libraries

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

LocalLoadingCache.get(K, Supplier) wastes an instance on every call, even if the key is present #1186

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
My particular use case is:

I have single shared Service instance with LoadingCache, accessible from 
multiple threads. Service itself does not have access to DataSource, so I 
cannot create CacheLoader in Service and provide it to LoadingCache - instead 
I'am using overloaded "get" method of Cache: V get(K key, Callable<? extends V> 
valueLoader);

My concern is: every time I call the method with Callable - LocalCache 
implementation creates new CacheLoader (wrapper object for that Callable), even 
if Cache already has value for that key. It is not very Garbage Collector 
friendly.

LocalCache has a solution for that, already - method that accepts CacheLoader 
as second argument. Unfortunately, it is package private and cannot be accessed 
from outside.

It would be beneficial to expose this method via LoadingCache interface.

Original issue reported on code.google.com by Alexey.M...@gmail.com on 2 Nov 2012 at 12:05

GoogleCodeExporter commented 9 years ago 
Do you actually have profiling data demonstrating that these CacheLoader 
objects represent a performance problem for your application?  Allocating 
cheap, short-lived objects is not generally a problem for most JVMs these days.

Original comment by wasserman.louis on 2 Nov 2012 at 4:05

GoogleCodeExporter commented 9 years ago 
No, I have not.
I was just thinking, if it's possible (and relatively effortless) to avoid 
extra object allocation - it's always better to do so.

But I agree with you - I need to do profiling.
I will build custom version of guava (with method exposed via interface) and 
will share results of A-B profiling.

Original comment by Alexey.M...@gmail.com on 2 Nov 2012 at 5:54

GoogleCodeExporter commented 9 years ago 
Messing around with the Cache and LoadingCache APIs at this point is far from 
effortless, I'm afraid -- hence our need for hard data to justify this.

Original comment by wasserman.louis on 2 Nov 2012 at 8:56

GoogleCodeExporter commented 9 years ago 
Seems slightly nice if this wasted instance can be avoided.

Original comment by kevinb@google.com on 2 Nov 2012 at 9:55

GoogleCodeExporter commented 9 years ago 
This can be done without an API change by more or less copypasting the internal 
implementation for CacheLoaders to Callables.  Do we think this is likely worth 
the code bloat?

Original comment by lowas...@google.com on 7 Nov 2012 at 6:50

GoogleCodeExporter commented 9 years ago 
I don't mind having some redundant code in our implementation at all.  Might as 
well save these instances.

Original comment by kevinb@google.com on 9 Nov 2012 at 11:47

GoogleCodeExporter commented 9 years ago 
Kevin and I talked about it and it looks like this is more duplication than 
we're willing to accept.

Original comment by lowas...@google.com on 10 Nov 2012 at 12:14

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

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

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

GoogleCodeExporter commented 9 years ago

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