Use Caffeine instead of Guava for caching. #7346
Conversation
Signed-off-by: Lukas Mansour <lukas.mansour@desy.de>
Signed-off-by: Lukas Mansour <lukas.mansour@desy.de>
Signed-off-by: Lukas Mansour <lukas.mansour@desy.de>
Signed-off-by: Lukas Mansour <lukas.mansour@desy.de>
…nt' to use Caffeine. Signed-off-by: Lukas Mansour <lukas.mansour@desy.de>
…dav', 'gplamza-oidc', 'srm-server' and 'dache-qos' to use Caffeine. Signed-off-by: Lukas Mansour <lukas.mansour@desy.de>
|
ok to test |
Signed-off-by: Lukas R. Mansour <lukas.mansour@desy.de>
Signed-off-by: Lukas Mansour <lukas.mansour@desy.de>
LukasMansour
force-pushed
the
caffine-switch
branch
from
f948727
to
9864bc5
Compare
| nfsClient = context.getStateHandler().getClientIdByStateId(stateid); | ||
| } | ||
|
|
||
| final NFS4State state = nfsClient.state(stateid); |
There was a problem hiding this comment.
the sateid in this context is the key, isn't it?
There was a problem hiding this comment.
Yes it is, however I do not see any significant advantage (one can argue performance benefits for either or) of using the lambda parameter or the parameter from the higher scope. The idea is that you could use a getter or a return method in the statement and don't need to store it another variable
Advantage in the following case:
cache.get(someinstance.getStringIDontWantToStoreInAVariable(), (string) -> "defaultValue" + string)No real advantage in the following case (our case):
public getFromCacheOrDefault(String key) {
cache.get(key, (key_) -> "defaultValue" + key_) // I could just as easily use key and not key_ here.
}Maybe I am mistaken, but I thought in this case it would be easier to just wait for JDK 21 where we have unnamed variables ("_"), hence I just called it key.
| @@ -217,8 +218,8 @@ public byte[] readBytes(Token token) { | |||
| */ | |||
| private Token makeToken(long id) { | |||
| try { | |||
| return canonicalizationCache.get(id, () -> new Token(id)); | |||
| } catch (UncheckedExecutionException | ExecutionException e) { | |||
| return canonicalizationCache.get(id, (key) -> new Token(id)); | |||
There was a problem hiding this comment.
same here (if I correctly read caffeine API)
Signed-off-by: Lukas Mansour <lukas.mansour@desy.de>
|
retest this please |
|
@LukasMansour could you please rebase to the current master |
Motivation:
Work towards #4087
Modification:
Replace's Guava's caching with Caffeine. Caffeine is the google-supported alternative to guava's caches, that importantly uses CompletableFutures instead of ListenableFutures. Guava does not plan to ever add this support and only does bug-fixes for Guava's caches.
It's important to remember, that the 'async' terminology used in Guava/Caffeine varies from the JDK's 'async' terminology. As such some issues may arise during translation from one library to the other. But for most cases, one will see that the translation is very straight-forward.
I recommend this for the release after 9.2.
Result:
Replaces all uses of Guava's cache with Caffeine's cache and #4087 is now feasible without having to wrap ListenableFutures in CompletableFutures. With these commits It's possible to completely remove any import on ListenableFuture, this is however not done yet in this commit.
Signed-off-by: Lukas Mansour lukas.mansour@desy.de