RFR: 8293170: Improve encoding of the debuginfo nmethod section [v6]

Evgeny Astigeevich eastigeevich at openjdk.org
Tue Oct 25 09:31:34 UTC 2022


On Mon, 24 Oct 2022 22:05:13 GMT, Evgeny Astigeevich <eastigeevich at openjdk.org> wrote:

>> Storing the debug info is an iterative process. Chunks of data are compared to avoid duplication, and on some points the generated data is discarded and position is unrolled. Besides read/write stream implementation internals, DebugInformationRecorder uses raw stream data access to track the similar chunks (see [DIR_Chunk](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/code/debugInfoRec.cpp#L57)) and [memcpy](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/code/nmethod.cpp#L1969) the raw data. We have either to (1) align the data on positions where the DebugInformationRecorder splits data into chunks or to (2) take the bit position into account.
>> 
>> I experimented with `int position` to contain both the stream bit position in the least significant bits and stream byte position in most significant bits. For me the code becomes less readable and the performance is questionable even without the DebugInformationRecorder update:
>> 
>> -  uint8_t b1 = _buffer[_position] << _bit_pos;
>> -  uint8_t b2 = _buffer[++_position] >> (8 - _bit_pos);
>> +  uint8_t b1 = _buffer[_position >> 3] << (_position & 0x7);
>> +  _position += 8;
>> +  uint8_t b2 = _buffer[_position >> 3] >> (8 - _position & 0x7);
>> 
>> I would avoid this change and stay with current implementation. In fact, there is not much aligned positions within the data. And `assert(_stream->position() > serialized_null, "sanity");` (thanks for noticing that!) in the constructor makes no problem because data is aligned at the beginning of the stream.
>
> If we introduce `flush()` we can explicitly call it before any accesses to the internal buffer: `buffer()` and `position()`. We will add asserts to `buffer()` and `position()` checking whether there is unflushed data. I always prefer being explicit.

I think I have an idea of a solution which does not need `flush()` and will have the read-only `position()`.

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

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


More information about the hotspot-compiler-dev mailing list