RFR: 8324129: C2: Remove some ttyLocker usages in preparation for JDK-8306767
Emanuel Peter
epeter at openjdk.org
Fri Jan 19 08:06:30 UTC 2024
On Fri, 19 Jan 2024 04:25:45 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> src/hotspot/share/interpreter/bytecodeTracer.cpp line 195:
>>
>>> 193: s.set_interval(from, to);
>>> 194:
>>> 195: ttyLocker ttyl; // keep the following output coherent
>>
>> The previous method using ttyLocker states:
>>
>> // The ttyLocker also prevents races between two threads
>> // trying to use the single instance of BytecodePrinter.
>>
>> Is that also a concern in this method?
>
> FTR the code should not be assuming that `st` is `tty`!
> Is that also a concern in this method?
I don't think so, since `BytecodeTracer::print_method_codes` has its own local instance of `BytecodePrinter`, whereas `BytecodeTracer::trace_interpreter` uses the global instance `_interpreter_printer`.
Using the `ttyLocker` for mutual exclusion on `_interpreter_printer` seems a bit ugly, and the comment seems to suggest as much. We can fix that in the future, if we want.
> FTR the code should not be assuming that st is tty!
Why are you saying that? I guess, yes, the code was already suspicios, since `st` was not guaranteed to be `tty`. But taking the `ttyLocker` was kinda ok if it was tty or now. Anyway, it is better if it is gone now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17486#discussion_r1458539949
More information about the hotspot-dev
mailing list