Enable recursive updates to NamespacedHierarchicalStore by mpkorstanje · Pull Request #5231 · junit-team/junit-framework (original) (raw)

@mpkorstanje @martinfrancois

Co-authored-by: martinfrancois f.martin@fastmail.com

@mpkorstanje

@mpkorstanje @martinfrancois

Co-authored-by: martinfrancois f.martin@fastmail.com

@mpkorstanje

@mpkorstanje

@mpkorstanje

@mpkorstanje

@mpkorstanje

@mpkorstanje mpkorstanje changed the titleEnable store recursive updates Enable recursive updates to NamespacedHierarchicalStore

Dec 15, 2025

@mpkorstanje

@mpkorstanje

@mpkorstanje

marcphilipp

@mpkorstanje @marcphilipp

…support/store/NamespacedHierarchicalStore.java

Co-authored-by: Marc Philipp mail@marcphilipp.de

@mpkorstanje mpkorstanje deleted the rien/enable-store-recursive-updates branch

December 22, 2025 19:47

marcphilipp pushed a commit that referenced this pull request

Dec 28, 2025

@mpkorstanje @marcphilipp

Enable recursive updates to NamespacedHierarchicalStore

Users of the extension context may occasionally want to build a non-trivial object graph, of which the individual components are also stored in the extension context store. This naturally leads to a pattern where the store is updated recursively. For example:

store.getOrComputeIfAbsent(("a", __ -> {
  B b = store.getOrComputeIfAbsent("b", _ -> new B(), B.class);
  C c = store.getOrComputeIfAbsent(("c", _ -> new C(), C.class);
  return new A(b, c);
}, A.class);

While the backing ConcurrentHashMap does not support recursive updates this does work as getOrComputeIfAbsent defers the recursive update until the after the map has been updated. This is not the case for computeIfAbsent which immediately calls the default creator. This poses a problem as computeIfAbsent is supposed to be a drop-in replacement for getOrComputeIfAbsent with better null and exception semantics.

The solution is to defer the execution of the default value creator until after the map has been updated. This is handled by the DeferredSupplier which uses a FutureTask to support a nice separation of compute and get operations.

Because there are some differences between both compute methods in how deferred values are to be handled, this PR introduces a StoredValue.Value, .DeferredValue and .DeferredOptionalValue containers for put, getOrComputeIfAbsent and computeIfAbsent respectively. This allows:

This PR does introduce a small change in behavior. Previously any of the concurrent callers of getOrComputeIfAbsent could be the one executing the default creator. This can now only be done by the caller that created the stored value that was actually inserted. I don't expect this will break anything.

The PR is also slightly bigger than it needs to be. StoredValue.Value could be replaced with .DeferredValue. But as DeferredValue only exists to support a deprecated method, this makes for easier clean up in the future.

Thanks to Martin Francois who contributed the CollidingKey, DeferredSupplier and several other useful concepts in #5209.

Fixes: #5171

(cherry picked from commit 5a3d882)

mpkorstanje added a commit that referenced this pull request

Feb 11, 2026

@mpkorstanje @marcphilipp

In #5231 evaluation of the default value creator was deferred until after an entry was created in the concurrent hash map backing the store. This enables recursive updates. However, by calling evaluateIfNotNull inside compute threads that lost the race would wait inside Map::compute for the value to be evaluated.

Waiting here is a problem when the backing concurrent hashmap is reaching capacity. The thread waiting inside compute holds a lock while at the same time the thread that won the race will try to acquire a lock to resize the map.

By making the threads that lost the race wait outside of Map::compute we can prevent this dead lock. This does require that each thread retries when another thread failed to insert a value. But eventually either one other thread either successfully inserts a value or the thread uses its own default value creator.

Fixes: #5346

Co-authored-by: Marc Philipp mail@marcphilipp.de

marcphilipp pushed a commit that referenced this pull request

Feb 15, 2026

@mpkorstanje @marcphilipp

In #5231 evaluation of the default value creator was deferred until after an entry was created in the concurrent hash map backing the store. This enables recursive updates. However, by calling evaluateIfNotNull inside compute threads that lost the race would wait inside Map::compute for the value to be evaluated.

Waiting here is a problem when the backing concurrent hashmap is reaching capacity. The thread waiting inside compute holds a lock while at the same time the thread that won the race will try to acquire a lock to resize the map.

By making the threads that lost the race wait outside of Map::compute we can prevent this dead lock. This does require that each thread retries when another thread failed to insert a value. But eventually either one other thread either successfully inserts a value or the thread uses its own default value creator.

Fixes: #5346

Co-authored-by: Marc Philipp mail@marcphilipp.de

(cherry picked from commit f29de38)

marcphilipp pushed a commit that referenced this pull request

Feb 15, 2026

@mpkorstanje @marcphilipp

In #5231 evaluation of the default value creator was deferred until after an entry was created in the concurrent hash map backing the store. This enables recursive updates. However, by calling evaluateIfNotNull inside compute threads that lost the race would wait inside Map::compute for the value to be evaluated.

Waiting here is a problem when the backing concurrent hashmap is reaching capacity. The thread waiting inside compute holds a lock while at the same time the thread that won the race will try to acquire a lock to resize the map.

By making the threads that lost the race wait outside of Map::compute we can prevent this dead lock. This does require that each thread retries when another thread failed to insert a value. But eventually either one other thread either successfully inserts a value or the thread uses its own default value creator.

Fixes: #5346

Co-authored-by: Marc Philipp mail@marcphilipp.de

(cherry picked from commit f29de38)

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})