RFR: JMC-7090: [JFR Writer] Make constant pool optional when registering a new type

Alexander Kuznetsov github.com+13893590+alexvangogen at openjdk.java.net
Mon Jan 18 20:33:40 UTC 2021


On Mon, 18 Jan 2021 15:02:02 GMT, Jaroslav Bachorik <jbachorik at openjdk.org> wrote:

> JMC-7090: [JFR Writer] Make constant pool optional when registering a new type

@jbachorik Thanks for this contribution! 
One additional thought: wouldn’t it be simpler to just add something like `TypeStructureBuilder#withConstantPool()` instead of extending API with additional flag parameter?

core/org.openjdk.jmc.flightrecorder.writer/src/main/java/org/openjdk/jmc/flightrecorder/writer/TypesImpl.java line 151:

> 149: 	@Override
> 150: 	public TypeImpl getOrAdd(Predefined type, Consumer<TypeStructureBuilder> builderCallback) {
> 151: 		return getOrAdd(type.getTypeName(), false, builderCallback);

Just curious, why `withConstantPool` is `false` by default? It alters the previous solution where every jdk type except `jdk.types.Event` was considered to have constant pool, thus causing `ChunkComplexTest` to fail here: https://github.com/openjdk/jmc/blob/2b1af4140b207411a08fde5467b94d86333c1f4a/core/org.openjdk.jmc.flightrecorder.writer/src/test/java/org/openjdk/jmc/flightrecorder/writer/ChunkComplexTest.java#L130
Without falling to constant pool, stack traces and stack frames are written in-place, and since `classLoader` is not specified in test, NPE is thrown:
https://github.com/openjdk/jmc/blob/2b1af4140b207411a08fde5467b94d86333c1f4a/core/org.openjdk.jmc.flightrecorder.writer/src/main/java/org/openjdk/jmc/flightrecorder/writer/TypedValueImpl.java#L133

core/org.openjdk.jmc.flightrecorder.writer/src/main/java/org/openjdk/jmc/flightrecorder/writer/TypesImpl.java line 161:

> 159: 	@Override
> 160: 	public TypeImpl getOrAdd(Predefined type, String supertype, Consumer<TypeStructureBuilder> builderCallback) {
> 161: 		return getOrAdd(type.getTypeName(), supertype, false, builderCallback);

Same question as above

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

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


More information about the jmc-dev mailing list