Simplify CodecCache
#1018
Simplify CodecCache
#1018
Conversation
JAVA-4751
JAVA-4751
JAVA-4751
| public synchronized <T> Codec<T> putIfMissing(final CodecCacheKey codecCacheKey, final Codec<T> codec) { | ||
| Optional<Codec<?>> cachedCodec = codecCache.computeIfAbsent(codecCacheKey, clz -> Optional.of(codec)); | ||
| if (cachedCodec.isPresent()) { | ||
| return (Codec<T>) cachedCodec.get(); | ||
| } | ||
| codecCache.put(codecCacheKey, Optional.of(codec)); | ||
| return codec; | ||
| } |
There was a problem hiding this comment.
Instead of improving this method as I originally described here, I did more changes, further simplifying the cache. Before this PR we were caching failures of finding a codec as Optional.empty() values just for the sake of throwing an exception faster if asked for the same codec again. I see no point in trying to optimize that path.
| def codec = Mock(Codec) | ||
| def provider = new CodecProvider() { | ||
| private int counter = 0 | ||
|
|
||
| @Override | ||
| Codec get(final Class clazz, final CodecRegistry registry) { | ||
| if (counter == 0) { | ||
| counter++ | ||
| return codec | ||
| } | ||
| throw new AssertionError((Object)'Must not be called more than once.') | ||
| } | ||
| } |
There was a problem hiding this comment.
For some reason, I failed to use Spock stubbing here (the stabbed provider works as expected if used directly from the test, but does not work when used via registry. So I had to stub the object manually.
| return codecCache.<T>get(codecCacheKey).orElseGet(() -> { | ||
| for (CodecProvider provider : codecProviders) { | ||
| Codec<T> codec = provider.get(context.getCodecClass(), context); | ||
| if (codec != null) { | ||
| if (codec instanceof Parameterizable && context.getTypes().isPresent()) { | ||
| codec = (Codec<T>) ((Parameterizable) codec).parameterize(context, context.getTypes().get()); | ||
| } | ||
| return codecCache.putIfMissing(codecCacheKey, codec); | ||
| return codecCache.putIfAbsent(codecCacheKey, codec); |
There was a problem hiding this comment.
I have just realized that this whole thing can and should be done as a single compute. Will do.
There was a problem hiding this comment.
Turns out, IllegalStateException: Recursive update is a thing despite the map being concurrent :( Well, we can't have all the good things.
JAVA-4751
This reverts commit 12528a8.
Yes. While |
JAVA-4751