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

[Christoph Langer]

On Mon, 30 May 2022 19:09:28 GMT, Jaroslav Bachorik <jbachorik at openjdk.org> wrote:

>> This change allows the JMC parser users to consume frames with the type set to non-standard values (eg. from async-profiler).
>> 
>> The hard-wired enum approach is replaced with a hybrid solution keeping the standard types in enum-like structure and using a cache for the non-standard types. This allows being fairly flexible when dealing with new frame types while keeping full backward compatibility.
>
> Jaroslav Bachorik has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Use LRU frame type cache

Good idea to use LRU cache here. I didn't even know about these capabilities of LinkedHashMap yet. ��
I have some minor suggestions but other than that, looks good. Maybe you also want to merge with master to get green GHAs.

core/org.openjdk.jmc.common/src/main/java/org/openjdk/jmc/common/IMCFrame.java line 36:

> 34: package org.openjdk.jmc.common;
> 35: 
> 36: import java.util.HashMap;

Now the java.util.HashMap import isn't needed any more.

core/org.openjdk.jmc.common/src/main/java/org/openjdk/jmc/common/IMCFrame.java line 102:

> 100: 		 */
> 101: 		private static final int TYPE_CACHE_MAX_SIZE = 100;
> 102: 		/*

Please add a blank line here.

core/org.openjdk.jmc.common/src/main/java/org/openjdk/jmc/common/IMCFrame.java line 108:

> 106: 		 */
> 107: 		private static final Map<String, Type> TYPE_CACHE = new LinkedHashMap<String, Type>() {
> 108: 			@Override

Eclipse shows a warning about a missing serialVersionUID - maybe you'd want to add one?

core/org.openjdk.jmc.common/src/main/java/org/openjdk/jmc/common/IMCFrame.java line 109:

> 107: 		private static final Map<String, Type> TYPE_CACHE = new LinkedHashMap<String, Type>() {
> 108: 			@Override
> 109: 			protected boolean removeEldestEntry(Map.Entry eldest) {

Should be: `protected boolean removeEldestEntry(Map.Entry<String, Type> eldest) {`

core/tests/org.openjdk.jmc.flightrecorder.test/src/main/java/org/openjdk/jmc/flightrecorder/test/internal/util/ParserToolkitTest.java line 45:

> 43: 	public void testParseBuiltinFrameType() {
> 44: 		Assert.assertTrue(IMCFrame.Type.INTERPRETED == ParserToolkit.parseFrameType(ParserToolkit.INTERPRETED_TYPE_ID));
> 45: 		Assert.assertTrue(

Make one line out of L45 & L46? (Although the IDE probably did the line break...)

-------------

Changes requested by clanger (Committer).

PR: https://git.openjdk.java.net/jmc/pull/401