RFR: 8330051: Small ObjectMonitor spinning code cleanups [v5]

[Coleen Phillimore]

On Fri, 19 Apr 2024 11:58:32 GMT, Fredrik Bredberg <fbredberg at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Better name for enum for trylock interference.
>
> src/hotspot/share/runtime/objectMonitor.cpp line 986:
> 
>> 984:     if (TrySpin(current)) break;
>> 985: 
>> 986:     {
> 
> This is really a general comment.
> I don't like when `break`/`continue`/`return`/`goto` is on the same line as the `if` statement.
> Mostly because it makes it very hard to put a breakpoint that triggers on the `break`/`continue`/`return`/`goto`.
> In this case (line 984) it's also a bit of a readability issue since you might think that the local block starting at line 986 belongs to the `if` statement, until you see the `break`.
> I would prefer if the `break` is put on a line of it's own.
> There are many lines like this is the code base and I don't think we should change all, but I do think that we could change it when we do other changes to such a line.

I agree with you.  It makes the control flow harder to understand in these places.  I fixed the ones on lines that I changed with this PR.  Thanks for the comment!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18730#discussion_r1572296279