Do not synchronize on ClassCache operations #540
Conversation
jbachorik
force-pushed
the
jb/lockfree_classcache
branch
from
6f0eed2 to
c896518
Compare
| return infos; | ||
| } | ||
|
|
||
| private static String getClKey(ClassLoader cl) { | ||
| return cl.getClass().getName() + "@" + System.identityHashCode(cl); |
There was a problem hiding this comment.
Think use Pair is better than concatenate string for key
There was a problem hiding this comment.
Yes, this wold be the right thing to do. Once I have your confirmation that this actually resolves your problems I will polish this PR.
Anyway, thanks for chiming in! Not a lot of feedback on PRs here usually ...
| if (cl == null) { | ||
| return bootstrapInfos; | ||
| } | ||
| Map<ClassName, ClassInfo> infos = cacheMap.computeIfAbsent(cl, k -> new HashMap<>(500)); | ||
| boolean[] rslt = new boolean[] {false}; |
There was a problem hiding this comment.
This might be questionable - atomic boolean adds the overhead of CAS handling and since the value is updated and read from the same thread we don't need to enforce the visibility of the changed value and the final array might be fine.
TBH, not sure what the diff in the performance would be (if any) without profiling first but I used to use this pattern when multithreaded access was not required.
There was a problem hiding this comment.
agreed, though i am more curious why boolean[] instead of boolean?
There was a problem hiding this comment.
The flag is changed from within a lambda. For that the variable needs to be effectively final.
If it were just a boolean then being effectively final would prevent changing the value.
A final, single element array is working around this limitation.
jbachorik
force-pushed
the
jb/lockfree_classcache
branch
from
8b9f041 to
bca084f
Compare
See #539