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

[Boris Ulasevich]

On Thu, 22 Sep 2022 21:18:48 GMT, Evgeny Astigeevich <eastigeevich at openjdk.org> wrote:

>> Boris Ulasevich has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   cleanup
>
> src/hotspot/share/code/compressedStream.cpp line 152:
> 
>> 150: }
>> 151: 
>> 152: int CompressedSparseDataWriteStream::position() {
> 
> The function with a side effect looks strange to me. I see an assert in `DebugInformationRecorder::DebugInformationRecorder(OopRecorder* oop_recorder)` which uses it for checking.  So the assert can cause side affects. I am not sure it is expected.

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.

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

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