On Thu, 16 Jan 2025 14:16:15 GMT, Matthias Ernst <[email protected]> wrote:
>> Per Minborg has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove unused class
>
> src/java.base/share/classes/jdk/internal/util/SingleElementPool.java line 132:
>
>> 130: @ForceInline
>> 131: private boolean takePooledElement() {
>> 132: return UNSAFE.getAndSetInt(this,
>> POOLED_ELEMENT_TAKEN_OFFSET, TRUE) == FALSE;
>
> Does this need to use atomics? Looking at similar code in
> sun.nio.ch.Util.BufferCache which seems to be working without. My
> interpretation is that BufferCache relies on a virtual thread to not get
> preempted inside the { CTL.get().takeElement } sequence? Would the same
> reasoning not work here?
>
> I.e.
>
> element = carrierThreadLocal.get() [1]
> if (element.x) [2] a) can this virtual thread be
> moved to another carrier on [2] or [3]
> element.y [3] b) can this virtual thread be
> suspended and another one schedule between [1] and [2]/[3]?
>
>
> My understanding is that in the current implementation of virtual threads,
> this does not happen and we can manage the cache element with straight
> non-atomic code (correct)? If not correct, how does the BufferCache manage
> (pinned somewhere?)?
Hah, I have now discovered `Continuation.pin`. I believe with that this cache
would better use pinned sections around the CTL elements and in return not
require any atomics.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22391#discussion_r1918800055
