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

[Boris Ulasevich]

On Tue, 13 Dec 2022 13:23:56 GMT, Evgeny Astigeevich <eastigeevich at openjdk.org> wrote:

>> Boris Ulasevich has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 17 additional commits since the last revision:
>> 
>>  - minor api refactoring: start_scope and roll_back instead of position and set_position
>>  - buffer() returns const array
>>  - cleanup, rename
>>  - warning fix
>>  - add test for buffer grow
>>  - adding jtreg test for CompressedSparseDataReadStream impl
>>  - align java impl to cpp impl
>>  - rewrite the SparseDataWriteStream not to use _curr_byte
>>  - introduce and call flush() excplicitly, add the gtest
>>  - minor renaming. adding encoding examples table
>>  - ... and 7 more: https://git.openjdk.org/jdk/compare/f4a674b2...e9269942
>
> src/hotspot/share/code/compressedStream.hpp line 121:
> 
>> 119: 
>> 120: public:
>> 121:   CompressedSparseData(int position = 0) {
> 
> As it is one-argument constructor, let it be `explicit CompressedSparseData`.

ok

> src/hotspot/share/code/compressedStream.hpp line 201:
> 
>> 199:   // Start grouped data. Return a byte offset position in the stream where grouped data begins
>> 200:   int start_scope() {
>> 201:     align(); // a side effect!
> 
> I think we don't need the comment here.

sure

> src/hotspot/share/code/compressedStream.hpp line 209:
> 
>> 207:     _position = pos;
>> 208:     _bit_position = 0;
>> 209:     assert(_position < _size, "set_position is only used for rollback");
> 
> Should we change the assert and move it to the beginning of the function?
> 
> assert(pos <= _position, "new position must be rollback the current position"

right. thank you

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

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