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

[Evgeny Astigeevich]

On Wed, 14 Dec 2022 06:22:48 GMT, Boris Ulasevich <bulasevich at openjdk.org> wrote:

>> The nmethod "scopes data" section is 10% of the size of nmethod. Now the data is compressed using the Pack200 algorithm, which is good for encoding small integers (LineNumberTable, etc). Using the fact that half of the data in the partition contains zeros, I reduce its size by another 30%.
>>  
>> Testing: jtreg hotspot&jdk, Renaissance benchmarks
>
> Boris Ulasevich has updated the pull request incrementally with one additional commit since the last revision:
> 
>   a few minor changes

src/hotspot/share/code/compressedStream.cpp line 119:

> 117: bool CompressedSparseDataReadStream::read_zero() {
> 118:   if (_buffer[_position] & (1 << (7 - _bit_position))) {
> 119:     return 0; // not a zero data

As the return type is `bool`, let's use `false` and 'true` instead of `0` and `1`.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/CompressedSparseDataReadStream.java line 34:

> 32:   }
> 33: 
> 34:   int bit_pos = 0;

Should we use the Java naming convention here?
Also, should we follow the Java Code Style? I see the indent style is C++: 2 spaces.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/CompressedSparseDataReadStream.java line 36:

> 34:   int bit_pos = 0;
> 35: 
> 36:   protected short buffer(int position) {

Private?

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/CompressedSparseDataReadStream.java line 40:

> 38:   }
> 39: 
> 40:   public byte readByteImpl() {

Should it be private?

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.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/code/CompressedSparseDataReadStream.java line 62:

> 60:   }
> 61: 
> 62:   boolean readZero() {

Private?

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`.

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

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