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