RFR: 8292591: Experimentally add back barrier-less Java thread transitions [v5]

[Daniel D. Daugherty]

On Fri, 9 Sep 2022 17:39:35 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> src/hotspot/os/linux/systemMemoryBarrier_linux.cpp line 51:
>> 
>>> 49:   MEMBARRIER_CMD_PRIVATE_EXPEDITED          = (1 << 3),
>>> 50:   MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED = (1 << 4),
>>> 51: };
>> 
>> Does there need to be some form of "#ifndef XXX" ... "#endif" bracketing
>> for when we eventually reach systems running kernel 4.14?
>
> The plan is when the build system fully supports AC_CHECK_DECL, auto conf will check this for and set a define.

Thanks for the explanation!

>> src/hotspot/share/runtime/handshake.cpp line 380:
>> 
>>> 378:   if (UseSystemMemoryBarrier) {
>>> 379:     SystemMemoryBarrier::emit();
>>> 380:   }
>> 
>> It's not clear to me why this emit() is here.
>
> add_operation() arms poll.
> try_process() reads JavaThread state.
> 
> Since are allowing the CPU to do reordering of instructions, try_process() can see e.g. state in_native after arming the poll.
> Normally this would be safe, but since we now allow reordering it's not.
> By emitting the system membar we know that if we see in_native that thread will see the armed poll.
> 
> EDIT:
> You can also see long comment from me above.
> But yes it's not clear that add_operation() and try_process() are effected by this race when using system membar.

How about a comment above the emit():
// Separate the poll() in add_operation() above() from the read of JavaThread state
// in the try_process() call below.

>> test/hotspot/jtreg/runtime/handshake/SystemMembarHandshakeTransitionTest.java line 57:
>> 
>>> 55:         commands.addAll(Arrays.asList(args));
>>> 56:         commands.add("HandshakeTransitionTest$Test");
>>> 57:         ProcessBuilder pb = ProcessTools.createTestJvm(commands);
>> 
>> Please make sure the new test is run with 'release' bits.
>
> Yes it does.

Thanks!

-------------

PR: https://git.openjdk.org/jdk/pull/10123