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

Boris Ulasevich bulasevich at openjdk.org
Thu Oct 13 09:38:31 UTC 2022


On Tue, 11 Oct 2022 15:17:24 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 198:
> 
>> 196:   if (nsize < min_expansion*2) {
>> 197:     nsize = min_expansion*2;
>> 198:   }
> 
> We will not need the code, if we initialise `_size` to `max2(initial_size, UNSIGNED5::MAX_LENGTH)` in the constructor. I don't think `initial_size` less than `UNSIGNED5::MAX_LENGTH` makes sense.
> 
> `grow()` is invoked when `_position >= _size`. So there are two cases:
> 1. `_position == _size`
> 2. `_position > _size`
> 
> `_position < 2 * _size` will be satisfied for case 1.
> How do you guarantee `_position < 2 * _size` for case 2?

I agree. Theoretically, one can call set_position() to something far outside the buffer capabilities, and get the write() to store data far beyond the buffer. As it does not happen really, let me add assert(nsize > _position, "sanity") here.

Also I remove (nsize < 2 * UNSIGNED5::MAX_LENGTH) check as the DebugInfo buffer [start size is 10K](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/code/debugInfoRec.cpp#L130).
By the way, 10K seems too big. It covers 999 cases out of 1000.

> src/hotspot/share/code/compressedStream.hpp line 119:
> 
>> 117:   u_char* _buffer;
>> 118:   int     _position; // current byte offset
>> 119:   size_t  _byte_pos {0}; // current bit offset
> 
> Is it a bit offset in the byte at `_position`?
> `_byte_pos` does not sound clear.

Yes. I rename _byte_pos to _bit_pos

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

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


More information about the hotspot-compiler-dev mailing list