Conversation
This works by starting virtual threads to do the object allocations, when the allocation concurrency is greater than one.
They are false positives because we can add methods to an interface when it is sealed and does not permit external implementations. See revapi/revapi#304
They now rely on count down latches to assert concurrent allocator calls, rather than relying on timing. This makes them more robust.
There was a problem hiding this comment.
Pull request overview
Adds support for faster pool population by allowing allocations (and related allocator work) to run concurrently, addressing issue #216.
Changes:
- Introduces allocator async hooks (
allocateAsync/deallocateAsyncandreallocateAsync) and updates timing adaptors to record latency for async operations. - Adds
PoolBuilder#setMaxConcurrentAllocations(...)(default 1) and wires it into the threaded allocation controller to run allocator work in parallel. - Expands/adjusts blackbox tests and testkits to exercise concurrent allocation/deallocation/reallocation and async failure propagation.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/stormpot/Allocator.java | Adds default async allocation/deallocation methods. |
| src/main/java/stormpot/Reallocator.java | Adds default async reallocation method. |
| src/main/java/stormpot/PoolBuilder.java | Adds configuration API for max concurrent allocations + docs. |
| src/main/java/stormpot/Pool.java | Javadoc typo fix. |
| src/main/java/stormpot/internal/PoolBuilderImpl.java | Stores/reads allocation concurrency setting and updates defaults/thread setup. |
| src/main/java/stormpot/internal/PoolBuilderDefaults.java | Adds default allocation concurrency field. |
| src/main/java/stormpot/internal/BAllocThread.java | Implements parallel allocator work and async completion handling. |
| src/main/java/stormpot/internal/Task.java | Extends task model to include async completions and switch signals. |
| src/main/java/stormpot/internal/AsyncAllocationCompletion.java | New task type for delivering async allocation results back to allocator thread. |
| src/main/java/stormpot/internal/AsyncDeallocationCompletion.java | New task type for delivering async deallocation completion back to allocator thread. |
| src/main/java/stormpot/internal/AllocatorSwitchRequestPending.java | New task type to wake allocator thread for allocator-switch processing. |
| src/main/java/stormpot/internal/ReallocatingAdaptor.java | Forwards async allocate/deallocate to underlying allocator. |
| src/main/java/stormpot/internal/TimingReallocatingAdaptor.java | Adds metrics recording for async allocate/deallocate. |
| src/main/java/stormpot/internal/TimingReallocatorAdaptor.java | Adds metrics recording for async reallocate. |
| src/main/java/stormpot/internal/IdentityHashSet.java | Tweaks insertion selection logic. |
| src/test/java/stormpot/tests/blackbox/ThreadBasedPoolTest.java | Adds concurrency/async propagation tests for threaded pools. |
| src/test/java/stormpot/tests/blackbox/ThreadBasedWithConcurrentAllocationPoolTest.java | New threaded-pool variant configured for concurrent allocations. |
| src/test/java/stormpot/tests/blackbox/AllocatorBasedPoolTest.java | Adjusts leak test mechanics and allocation-order test behavior. |
| src/test/java/testkits/AlloKit.java | Makes action sequencing thread-safe for concurrent test execution. |
| src/test/java/testkits/DelegateAllocator.java | New wrapper allocator used by async tests. |
| src/test/java/testkits/DelegateReallocator.java | New wrapper reallocator used by async tests. |
| api-changes.json | Revapi suppression entries for the new PoolBuilder interface methods. |
src/test/java/stormpot/tests/blackbox/ThreadBasedWithConcurrentAllocationPoolTest.java
Outdated
Show resolved
Hide resolved
…ble instead of exception Otherwise, an Error being thrown from the synchronous operations would cause the CompletableFuture to never be completed.
This setting is not supported by inline or direct allocation
…nNullPointerException
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (1)
src/main/java/stormpot/PoolBuilder.java:176
- The Javadoc for precise leak detection says it’s enabled by default (both here and in the setter docs), but
PoolBuilderTest.preciseLeakDetectionMustBeDisabledByDefaultasserts the default is disabled. Please align the documentation with the actual default behavior (or update the default if the docs are correct).
/**
* Return whether precise object leak detection is enabled, which is
* the case by default.
*
* @return {@code true} if precise object leak detection is enabled.
* @see #setPreciseLeakDetectionEnabled(boolean)
*/
boolean isPreciseLeakDetectionEnabled();
/**
* Enable or disable precise object leak detection. It is enabled by default.
* Precise object leak detection makes the pool keep an eye on the allocated
src/test/java/stormpot/tests/blackbox/ThreadBasedWithConcurrentAllocationPoolTest.java
Outdated
Show resolved
Hide resolved
…ctor deregistration has been completed
…nStages from completing
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allow the pool to populate faster by allocating multiple objects in parallel.
This adds asynchronous methods to
AllocatorandReallocatorfor their respective operations.The background allocation thread will use these if the pool has been configured with a
PoolBuilder.setMaxConcurrentAllocationsvalue greater than one.Default implementations of these methods are provided, which will spawn a virtual thread for each operation.
A few fixes to leak detection have also been implemented, due to shortcomings made apparent by the asynchronous allocator operations. For instance, reallocation previously did not re-register the reallocated object. There were also a possibility of false-positives, from the references to deallocated objects being cleared before their corresponding slot had been deregistered with the leak detector. A few flaky tests in that area have also been fixed.
This fixes #216