From: core-libs-dev <core-libs-dev-r...@openjdk.org> on behalf of Brian Goetz <briango...@openjdk.org>
Most entities in the system are immutable and therefore freely sharable without additional coordination. Some have lazily-inflated single-field caches that conform to the "benign race" (there is only one possible non-default value) criteria. The main hole is the CHA resolution cache. There are two ways to address this: protect the cache, or don't share the context. Unfortunately now that ClassModel refers back to the context, there's a risk of unexpected context sharing. Here are a few options: - Make the CHA cache thread-safe using, say, CHM; This is current solution for the default system CH cache. We may also remove ClassHierarchyResolver::cached() method to force user think about it and always call ClassHierarchyResolver::cached(Supplier<Map<ClassDesc, ClassHierarchyInfo>> cacheFactory) for all non-default cases. Or we may also use a bit less strict custom semi-synchronization (just enough to be thread safe) instead of fully-synchronized CHM::computeIfAbsent, for example CHM:get … <compute if absent without lock> ... CHM::put. - Make the CHA cache unshared, by using a ThreadLocal (this brings the context back to being immutable) This is interesting option for small number of threads, however it makes big footprint for heavy-parallel systems. - Detect when implicit use of a context happens on a thread other than the creating thread (currently the only vector for this is Classfile::transform), and inflate a new context with empty cache in that case; This would degrade the caching completely by triggering a new context for each transformed method in case there are two threads working synchronously. - Always create a fresh context with an empty cache in Classfile::transform This would destroy the caching. Such cache would work only in scope of one method. On 6/8/2023 9:24 AM, Adam Sotona wrote: > > Unfortunately thread-unsafe context makes sharing of it in tests > executed in parallel a nightmare. > I can fix our Corpus tests and hope the race condition won't raise > also somewhere else later. > However how to explain this limitation to users? > > I suggest to make it always thread-safe (as the context is primary > expected to be shared in multi-threaded environment) and users may > make it faster in specific cases by providing non-synchronized map. > > — > Reply to this email directly, view it on GitHub > <https://github.com/openjdk/jdk/pull/14180#issuecomment-1582576426>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AABJ4RHSMWR5Q3ANYK3RPSLXKHG7RANCNFSM6AAAAAAYQLACYQ>. > You are receiving this because you commented.Message ID: > ***@***.***> > ------------- PR Comment: https://git.openjdk.org/jdk/pull/14180#issuecomment-1582643016
