On Wed, 23 Nov 2022 19:16:23 GMT, Paul Sandoz <[email protected]> wrote:
> Looks good (just some last minor comments). I did not focus on the tests, nor
> too closely at all of the HotSpot changes.
>
> Something for future investigation perhaps (if not already thought about):
> consider using a persistent map, a Hash Array Mapped Trie (HAMT), for storing
> scoped keys and values, which could potentially remove the need for the cache
> of replace the cache when many values are in scope. The HAMT's structural
> sharing properties, wide-branching factor, and `Integer.bitCount` being
> intrinsic all make for an efficient implementation.
I've certainly considered a HAMT. (I've considered everything! It's been a long
journey.) The trouble is that using one penalizes binding operations because a
persistent HAMT requires path copying for insertions. This won't matter if your
bind operation is rare, such as at the start of a large block of code. However,
it might not be. Consider a common case such as a parallel stream using
fork/join.
void parallelRunWithCredentials(List<?> aList, Runnable aRunnable) {
var permissions = ScopedValue.where(PERMISSION, getCredentials());
aList.parallelStream().forEach(() -> permissions.run(aRunnable));
}
Because the binding operation `permissions.run()` is invoked for every element
on the list, we must make the binding operation as fast as it can possibly be.
In addition, the cache is _extremely_ fast. Typically it's four instructions,
and C2 almost always hoists values, once looked up in cache, into registers. A
HAMT lookup, while fast and (mostly) O(1), is more complex.
-------------
PR: https://git.openjdk.org/jdk/pull/10952
