RFR: 6996: Properly support converters

[Marcus Hirt]

On Thu, 26 Nov 2020 20:43:15 GMT, Gunnar Morling <github.com+28612+gunnarmorling at openjdk.org> wrote:

>> Including simple test to verify that the resulting instrumented methods are ok.
>
> agent/src/main/java/org/openjdk/jmc/agent/impl/ResolvedConvertable.java line 41:
> 
>> 39: 		}
>> 40: 		for (Method m : converterClass.getDeclaredMethods()) {
>> 41: 			if (CONVERTER_METHOD.equals(m.getName())) {
> 
> Should this not also take the parameter type into account? So to make sure multiple `convert()` methods for different types declared on the same class are invoked correctly based on assignability.

Sure. It will take the first assignable it can find though, not try to find the most specific. You'll have to specify the method explicitly to make that happen.

> agent/src/test/resources/org/openjdk/jmc/agent/converters/test/jfrprobes_template.xml line 86:
> 
>> 84: 						</description>
>> 85: 						<contenttype>None</contenttype>
>> 86: 						<converter>org.openjdk.jmc.agent.converters.test.GurkConverterInt
> 
> Thinking more about this, would it actually make more sense to specify a _method_ here? In the end, that's what we want to identify. That'd allow for usage of existing methods named other than `convert()`. WDYT?

I was considering it, since it would allow people to have multiple converters to different return types in the same class. It becomes a bit more cumbersome to specify though. Perhaps use convert as the default if only specified as a class, and the method, if specified?

> core/tests/org.openjdk.jmc.flightrecorder.rules.jdk.test/baseline/Generated_JfrRuleBaseline.xml line 1:
> 
>> 1: <?xml version="1.0" encoding="UTF-8" standalone="no"?>
> 
> What's the purpose of this and the next file?

This one was accidentally checked in. I'll remove it and add it to the .gitignore. Good catch.

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

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