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