RFR: JMC-7798: Make the frame type extensible [v2]
Jaroslav Bachorik
jbachorik at openjdk.java.net
Fri May 27 07:12:34 UTC 2022
On Thu, 19 May 2022 12:17:43 GMT, Christoph Langer <clanger at openjdk.org> wrote:
>> Jaroslav Bachorik has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Fix typo in javadoc
>
> core/org.openjdk.jmc.common/src/main/java/org/openjdk/jmc/common/IMCFrame.java line 66:
>
>> 64: String key = MSG_PREFIX + this.id;
>> 65: if (Messages.hasString(key)) {
>> 66: this.name = Messages.getString(key);
>
> Except for id, you could leave out `this` in the constructor
Done
> core/org.openjdk.jmc.common/src/main/java/org/openjdk/jmc/common/IMCFrame.java line 66:
>
>> 64: String key = MSG_PREFIX + this.id;
>> 65: if (Messages.hasString(key)) {
>> 66: this.name = Messages.getString(key);
>
> Except for id, you could leave out `this`
Done
> core/org.openjdk.jmc.common/src/main/java/org/openjdk/jmc/common/IMCFrame.java line 78:
>
>> 76: */
>> 77: public static final Type JIT_COMPILED = new Type("JIT_COMPILED"); //$NON-NLS-1$
>> 78: /**
>
> I would like if the static type definitions could move up, directly behind `final class Type {` and were separated by a blank line.
Moved
> core/org.openjdk.jmc.flightrecorder/src/main/java/org/openjdk/jmc/flightrecorder/internal/util/ParserToolkit.java line 89:
>
>> 87: * zero, so the memory overhead of the cache stays negligible.
>> 88: */
>> 89: private static final Map<String, IMCFrame.Type> TYPE_CACHE = new HashMap<>();
>
> I would suggest to move the Type Cache to be part of the Type implementation in IMCFrame.
Done
> core/tests/org.openjdk.jmc.flightrecorder.test/src/test/java/org/openjdk/jmc/flightrecorder/test/internal/util/ParserToolkitTest.java line 13:
>
>> 11: Assert.assertTrue(IMCFrame.Type.INTERPRETED == ParserToolkit.parseFrameType(ParserToolkit.INTERPRETED_TYPE_ID));
>> 12: Assert.assertTrue(
>> 13: IMCFrame.Type.JIT_COMPILED == ParserToolkit.parseFrameType(ParserToolkit.JIT_COMPILED_TYPE_ID));
>
> Maybe make one line out of L12 and L13
Fixed
-------------
PR: https://git.openjdk.java.net/jmc/pull/401
More information about the jmc-dev
mailing list