On Tue, 18 Feb 2025 10:54:13 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
>> Aleksey Shipilev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Revert waitForReferenceProcessing removals, see JDK-8305186
>
> src/java.base/share/classes/java/nio/Bits.java line 122:
>
>> 120: // Short on memory, with potentially many threads competing for
>> it.
>> 121: // To alleviate progress races, acquire the lock and go slow.
>> 122: synchronized (Bits.class) {
>
> Using a (somewhat) public object for this seems questionable. Why not a
> private lock object?
This class is package-private, so I judged the possibility of leaking the lock
impractically low. We can do a separate lock, if that reads cleaner, sure. Done
in new commit.
> src/java.base/share/classes/java/nio/Bits.java line 156:
>
>> 154: // Exponentially back off waiting for Cleaner to catch
>> up.
>> 155: try {
>> 156: Thread.sleep(sleepTime);
>
> There isn't a tryReserveMemory after the last sleep period, so that period is
> just completely wasted time. This is why the old code didn't use a for-loop
> over the sleep counter, but instead checked for reaching the limit in the
> middle of the loop.
Right, that's an overlook. I wanted to make the first non-sleep-ed attempt in
the loop, so that it covers the semi-optimistic case where we could have
reserved the memory immediately after taking the lock. But we can peel that out
from the loop. Done in new commit.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22165#discussion_r1959590847
PR Review Comment: https://git.openjdk.org/jdk/pull/22165#discussion_r1959590929
