Short circuit value cache look up to avoid object allocations #109
Conversation
… the value cache lookups
| @Override | ||
| public synchronized Throwable fillInStackTrace() { | ||
| return this; | ||
| } |
There was a problem hiding this comment.
This is magic - the exception has no stack trace (we don't want one) AND its fast to allocate because of this.
There was a problem hiding this comment.
No - it is in the base class. I will change this
| @Override | ||
| public synchronized Throwable fillInStackTrace() { | ||
| return this; | ||
| } |
| @@ -20,37 +22,55 @@ | |||
| @Internal | |||
There was a problem hiding this comment.
Maybe promote it to public? We expose a public static final NOOP which may be of use to library consumers
There was a problem hiding this comment.
The NOOP is a default impl for never caching. I want it to stay that way. There is no real useful code here honestly for library consumers
| /** | ||
| * a no op value cache instance | ||
| */ | ||
| public static final NoOpValueCache<?, ?> NOOP = new NoOpValueCache<>(); |
There was a problem hiding this comment.
If we make it public:
Introduce a private constructor? + Builder method for type safety?
|
Just realised that if we open it up (and make it final), people cannot reuse the magic values like |
Yeah I don't really want them do this on a @internal class - honestly if someone makes a ValueCache then we would expect them to NOT ever throw short circuit exceptions- I mean they can but unlikely |
|
I agree, thank you for the quick action :) |
The original PR : #108 shows we are allocating too many objects.
This takes it further by introducing a fast exception to allow a
ValueCacheto short circuit out of a look up.This is then used by the default NoopValueCache and hences saves memory and processing for every one NOT using a ValueCache