RFR: JMC-7798: Make the frame type extensible [v2]

[Jaroslav Bachorik]

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