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

[Marcus Hirt]

I just realized that the byte sizes are also defined in our DataInputToolkit, so will use those instead of redefining them.

/M

On 2018-05-21, 21:38, "jmc-dev on behalf of Erik Gahlin" <jmc-dev-bounces at openjdk.java.net on behalf of erik.gahlin at oracle.com> wrote:

    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
    >