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

[Boris Ulasevich]

On Wed, 14 Dec 2022 14:02:44 GMT, Evgeny Astigeevich <eastigeevich at openjdk.org> wrote:

>> Boris Ulasevich has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   a few minor changes
>
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/CompressedSparseDataReadStream.java line 40:
> 
>> 38:   }
>> 39: 
>> 40:   public byte readByteImpl() {
> 
> Should it be private?

ok

> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/CompressedSparseDataReadStream.java line 55:
> 
>> 53:     byte b = readByteImpl();
>> 54:     int result = b & 0x3f;
>> 55:     for (int i = 0; (0 != ((i == 0) ? (b & 0x40) : (b & 0x80))); i++) {
> 
> It is difficult to read this. Could we rewrite it into:
> 
> int result = b & 0x3f;
> if ((b & 0x40) != 0) {
>     b = readByteImpl();
>     result |= (b & 0x7f) << 6;
>     for (int i = 1; (b & 0x80) != 0; ++i) {
>           b = readByteImpl();
>           result |= ((b & 0x7f) << (6 + 7 * i));
>     }
> }
> 
> 
> BTW, we can simplify the loop in the `CompressedSparseDataReadStream::read_int()` in the same way.

If you don't mind, I would prefer shorter version.

for (int i = 0; (0 != ((i == 0) ? (b & 0x40) : (b & 0x80))); i++) {
  b = readByteImpl();
  result |= ((b & 0x7f) << (6 + 7 * i));
}

->

int result = b & 0x3f;
if ((b & 0x40) != 0) {
    b = readByteImpl();
    result |= (b & 0x7f) << 6;
    for (int i = 1; (b & 0x80) != 0; ++i) {
          b = readByteImpl();
          result |= ((b & 0x7f) << (6 + 7 * i));
    }
}

> test/hotspot/jtreg/serviceability/sa/TestCompressedSparseDataReadStream.java line 51:
> 
>> 49:         assertEquals(in.readInt(), 0);  // zero bit -> 0
>> 50:         in.setPosition(2);
>> 51:         assertEquals(in.readInt(), 48); // 0xf000   -> 48
> 
> Should we test `readBoolean` and `readByte`? I understand they are implemented with `readInt`. They might incorrectly convert data returned by `readInt`.

ok

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

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