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

Evgeny Astigeevich eastigeevich at openjdk.org
Wed Nov 16 01:01:14 UTC 2022


On Tue, 15 Nov 2022 23:31:51 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, rename
>
> 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.

I have an idea which might solve the issues.

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

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


More information about the hotspot-compiler-dev mailing list