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