RFR: Data plugin

[Leonid Kuskov]

On Tue, 26 Apr 2022 00:46:44 GMT, Alexandre Iline <shurailine at openjdk.org> wrote:

> * JREInstr
>  * instrumentation plugin enhancement
>  * build script
>  * Implantable interface
>  * support for all data types
>  * other minor enhancements

I  need more time for review. It's a first portion.

plugins/data_coverage/src/openjdk/jcov/data/Env.java line 63:

> 61:         Set<String> keys = System.getProperties().stringPropertyNames();
> 62:         keys.stream().filter(k -> k.startsWith(prefix))
> 63:                 .forEach(k -> System.clearProperty(k));

It's not good idea to clear/update system properties. Actually much better to use an "application" copy of them and interact with the copy as needed. Or after program shut down updated system properties could be used by an other application?

plugins/data_coverage/src/openjdk/jcov/data/Env.java line 66:

> 64:     }
> 65: 
> 66:     public static String getStringEnv(String property, String defaultValue) {

defaultValue isn't used and could be omited. 
line 70 of Class Collect  

if (!Env.getStringEnv(COVERAGE_IN, "").isEmpty()) {
``` 
could be chaged to 

if( Env.getStringEnv(COVERAGE_IN) != null

Also isn't clear why you wraps IOException with UncheckedIOException here.

plugins/data_coverage/src/openjdk/jcov/data/Env.java line 112:

> 110:                 String[] params = propValue.substring(ob + 1, cb).split(",");
> 111:                 Class[] paramTypes = new Class[params.length];
> 112:                 Arrays.fill(paramTypes, String.class);

Here the method MethodTypeDesc.ofDescriptor(String descriptor)could be used to parse method's descriptor if you use JVM notation for method descriptors like: (IDLjava/lang/Thread;)Ljava/lang/Object; for Object m(int i, double d, Thread t). Just in case if universal parser is needed here and params are not just Strings.

plugins/data_coverage/src/openjdk/jcov/data/JREInstr.java line 73:

> 71:                 "-template", jcovTemplate,
> 72:                 "-im", "java.base",
> 73:                 jre};

Is Objects.requireNonNull() needed?  It could be that jcovRuntime, jcovTemplate or pluginClass is null.

plugins/data_coverage/src/openjdk/jcov/data/arguments/instrument/MethodFilter.java line 50:

> 48:                 res.add(new Plugin.TypeDescriptor("[", "java/lang/Object", ALOAD));
> 49:                 if(desc.charAt(pos + 1) == 'L') pos = desc.indexOf(";", pos) + 1;
> 50:                 else pos = pos + 2;

Is the case [[[Ljava/lang/Object; covered?

plugins/data_coverage/src/openjdk/jcov/data/arguments/runtime/Collect.java line 105:

> 103: 
> 104:     static int countParams(String desc) {
> 105:         if(!desc.startsWith("(")) throw new IllegalArgumentException("Not a method descriptor: " + desc);

I'm not sure. But this parser should be updated with MethodTypeDesc.ofDescriptor && parameterCount() That will support all changes at method descriptors like Q references.

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

PR: https://git.openjdk.java.net/jcov/pull/19