On Wed, 3 Apr 2024 10:04:36 GMT, Alan Bateman <[email protected]> wrote:
> This change drops the adjustments to the virtual thread scheduler's target
> parallelism when virtual threads do file operations on files that are opened
> for buffered I/O. These operations are usually just too short to have any
> benefit and may have a negative benefit when reading/writing a small number
> of bytes. There is no change for read/write operations on files opened for
> direct I/O or when writing to files that are opened with options for
> synchronized I/O file integrity (O_SYNC/O_DSYNC and equivalents). Sergery
> Kuksenko is polishing benchmarks that includes this area, this is for a
> future PR.
>
> In addition, the blocker mechanism is updated to handle reentrancy as can
> happen if debugging code is added to ForkJoinPool, if there is preemption
> when attempting to compensate, or potentially forced preemption in the
> future. This part is a pre-requisite to the changes to better support object
> monitor there are more places where preemption is possible and this quickly
> leads to unbalanced begin/end.
>
> The changes have been baking in the loom repo for several months.
src/java.base/share/classes/jdk/internal/misc/CarrierThread.java line 81:
> 79: value = ForkJoinPools.beginCompensatedBlock(getPool());
> 80: } catch (Throwable e) {
> 81: if (compensating == COMPENSATE_IN_PROGRESS) {
I don't fully follow these checks `if (compensating == COMPENSATE_IN_PROGRESS)
{`. Surely it's not for concurrent access (given the `assert` at the start of
this method, this code path happens in a single thread). So I think these
checks are about the re-entrancy that is mentioned in the description of this
PR.
In the context of re-entrancy, if I am reading the code correctly, I don't see
how a re-entrant call would end up on this line (and other similar lines) in
this top level `if` block. When a thread enters the top level `if` block it
immediately sets the `compensating` to `COMPENSATE_IN_PROGRESS`:
if (compensating == NOT_COMPENSATING) {
compensating = COMPENSATE_IN_PROGRESS;
...
So a subsequent re-entrant call would never enter that top level `if` block
again. Which leads me to question the need of these additional `if
(compensating == COMPENSATE_IN_PROGRESS) {` checks. I feel like I am missing
something in this code.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18598#discussion_r1557834582
