RFR: Instrumentation plugin concept

[Alexandre Iline]

On Wed, 3 Mar 2021 22:04:51 GMT, Leonid Kuskov <lkuskov at openjdk.org> wrote:

>> These changes are introducing a concept of an instrumentation plugin into JCov. Instrumentation plugin (defined by _com.sun.tdk.jcov.instrument.InstrumentationPlugin_) represents a logic which specifies what code instrumentation is to be performed by JCov. 
>> 
>> In the current implementation, the concept is only used to define instrumentation **additional** to that which is already done by JCov. The instrumentation performed by such plugins may or may not be relates to the instrumentation already performed by JCov. As such, JCov could be used as only an engine to load and save the code, with the additional instrumentation having nothing to do with code coverage. 
>> 
>> It is also possible to reuse the same concept **within JCov itself**. Depending on the command line option, JCov already inserts very different instrumentations. This is now controlled programmatically such as in _com.sun.tdk.jcov.instrument.DeferringMethodClassAdapter.visitMethod(int, String, String, String, String[])_ method. Such code could possibly benefit from some delegation. This actual suggested change does not suggest any changes related to code instrumentation currently supported by JCov.
>> 
>> In addition to the instrumentation plugin, the changes suggest a way to specify a logic to save all data collected during the runtime. That is done by reusing _com.sun.tdk.jcov.runtime.JCovSaver_ interface.
>> 
>> 
>> To better understand the concept perhaps it is helpful to take a look on the test which is suggested as a part of this change: _com.sun.tdk.jcov.instrument.plugin.FieldsTest_ and the accompanying files. The same concept is explained in more details in this draft JEP proposal: https://bugs.openjdk.java.net/browse/JDK-8043291
>> 
>> 
>> More work needed.
>> 
>> Current state of the code is offered solely for the purpose of discussing the concepts of instrumentation plugin. When the concept is generally accepted, more changes to the source are needed and more testing is needed also. 
>> 
>> These are only some of the things which needs to be looked into:
>> 1. a mechanism is required to notify the plugin about the completion of the instrumentation process. This is needed for the plugin to be able to save data collected during instrumentation, similar to JCov saving a template. To do.
>> 2. what is the right way to pass the saver? Currently it is done through JCov properties.
>> 3. how to unit test the exit hook? See com.sun.tdk.jcov.runtime.Collect.init(), where there is a hack right now.
>> 4. how to test  instrumentation from the same VM? See _com.sun.tdk.jcov.lib.InstrProxy.instr(String[] , String...)_
>> 5. allow the instrumentation plugin through all entry points. Right now only _com.sun.tdk.jcov.Instr_ is supported and tested.
>
> Marked as reviewed by lkuskov (Committer).

@lkuskov , I have added another commit to support JREInstr.
I think this is in a good enough shape to be integrated into the main JCov repository. 
Would you agree to that?

The only functional change to the behavior, if no plugin is specified, is in com.sun.tdk.jcov.runtime.Collect.init():
https://github.com/openjdk/jcov/pull/9/files#diff-3ccc940162578203e7797c295946dcfdfe788c085bf2cf741b28c52609d2080d
I have not found a scenario where this change would be harmful. The change, however, needed for the tests to work.

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

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