RFR: 8338383: Implement JEP 491: Synchronize Virtual Threads without Pinning [v9]

Patricio Chilano Mateo pchilanomate at openjdk.org
Tue Nov 5 01:43:57 UTC 2024


On Tue, 5 Nov 2024 00:23:37 GMT, Fei Yang <fyang at openjdk.org> wrote:

>>> As the same code on aarch64 and x86-64 uses `frame::sender_sp_offset` I suggested to change the literal 2 into `frame::sender_sp_offset` in order to increase the readability, but I forgot that `frame::sender_sp_offset` is 0 on riscv64. However I do think it's a problem that several places throughout the code base uses a literal 2 when it should really be `frame::sender_sp_offset`. This type of code is very fiddly and I think we should do what we can to increase the readability, so maybe we need another `frame::XYZ` constant that is 2 for this case.
>> 
>> Yeah, I was also considering this issue when we were porting loom. I guess maybe `frame::metadata_words` which equals 2. Since this is not the only place, I would suggest we do a separate cleanup PR. 
>> 
>>> Also, does this mean that the changes from 2 to `frame::sender_sp_offset` in all of the lines (267, 271 and 273) should be reverted?
>> 
>> I agree with @pchilano in that we are fine with these places.
>
>> Sorry, I also thought it matched the aarch64 one without checking. @RealFYang should I change it for `hf.sp() + frame::link_offset` or just leave it as it was?
> 
> Or maybe `hf.sp() - frame::metadata_words`. But since we have several other occurrences, I would suggest we leave it as it was and go with a separate PR for the cleanup.

Reverted.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1828615499


More information about the core-libs-dev mailing list