Review Request: JMC-5922: Adding support for JDK9 and later for the chunk splitter.

Erik Gahlin erik.gahlin at oracle.com
Mon May 21 19:38:06 UTC 2018


Looks good.

Nits:

135: int index = lastChunkOffset + INTEGER_SIZE;

Should not JFR_MAGIC_BYTES.length be used instead of INTEGER_SIZE.

Also, there is Integer.BYTES and Short.Bytes that will give the number 
of bytes for the types.

263: long fullSize = DataInputToolkit.readLong(chunkHeader, HEADER_SIZE 
- LONG_SIZE);

I would define the position of chunk size relative to the start and what 
comes after the magic.

Also, JDK 8 and JDK 9 have different header sizes, so it's a bit unclear 
what is meant with HEADER_SIZE [1]

int CHUNK_SIZE_POSITION = JFR_MAGIC_BYTES.length;

DataInputToolkit.readLong(chunkHeader, CHUNK_SIZE_POSITION);

CHUNK_SIZE_POSITION could be used instead of INTEGER_SIZE above as well.

No need to create new webrev.

Erik

[1] 
http://hg.openjdk.java.net/jdk/jdk/file/tip/src/jdk.jfr/share/classes/jdk/jfr/internal/consumer/ChunkHeader.java

> Please review the fix for:
>
> JBS: https://bugs.openjdk.java.net/browse/JMC-5922
>
> Web rev: http://cr.openjdk.java.net/~hirt/JMC-5922/webrev.1/
>
>   
>
> Kind regards,
>
> Marcus
>



More information about the jmc-dev mailing list