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

[Evgeny Astigeevich]

On Tue, 15 Nov 2022 07:05:46 GMT, Boris Ulasevich <bulasevich at openjdk.org> wrote:

>> The nmethod "scopes data" section is 10% of the size of nmethod. Now the data is compressed using the Pack200 algorithm, which is good for encoding small integers (LineNumberTable, etc). Using the fact that half of the data in the partition contains zeros, I reduce its size by another 30%.
>>  
>> Testing: jtreg hotspot&jdk, Renaissance benchmarks
>
> Boris Ulasevich has updated the pull request incrementally with one additional commit since the last revision:
> 
>   cleanup, rename

src/hotspot/share/code/compressedStream.cpp line 193:

> 191:   if (_bit_position > 0) {
> 192:     grow_if_need();
> 193:     _buffer[_position] = (b << (8 - _bit_position));

I see many `grow_if_need` in code. This means if we forget to call it, we can have an issue.
What if we combine `grow_if_need` and `_buffer[_position] = ...` into one operation:

void put(u_char b) {
  grow_if_need();
  _buffer[_position] = b;
}

src/hotspot/share/code/compressedStream.hpp line 127:

> 125:   }
> 126: 
> 127:   u_char* buffer() const { return _buffer; }

The function changes access to `_buffer` from `protected` to `public`.
Should it be:

const u_char* buffer() const;

src/hotspot/share/code/compressedStream.hpp line 195:

> 193:     return _position;
> 194:   }
> 195:   void set_position(int pos) {

Now I see why we have a problem to implement `position` and `set_position`.
`position` originally had a meaning of the position where data would be written. Because of this it could be used to get the total amount of data written (see `DebugInformationRecorder::data_size`).
It was also used to mark a position to roll back later (e.g. `DebugInformationRecorder::serialize_scope_values`).
This violates the single-responsibility principle and makes difficult to add another implementation.
To restore the principle we need separate functionalities from `position` and `set_position` into something like:

// Mark the state of the stream.
void mark();

// Roll the stream state back to the marked one.  
void roll_back();

// Return the amount of data the stream contains.
int data_size();


We implement `mark` as creating copies of `_position`, `_bit_position` and `_buffer[_position]`. `roll_back` uses the copies to restore the state of the stream.
`CompressedSparseDataWriteStream::data_size()` just returns `_position + 1`.

There is the problem with `DebugInformationRecorder::find_sharable_decode_offset(int stream_offset)`. It calculates `stream_length` using `position()`. It depends too much on the current implementation. Because of this dependency we have to emulate it in our new implementation.

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

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