RFR: 8262881: port JVM/DI tests from JDK-4413752 to JVM/TI [v4]

Daniel D.Daugherty dcubed at openjdk.java.net
Wed Apr 7 19:20:40 UTC 2021


On Wed, 7 Apr 2021 17:52:16 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> Daniel D. Daugherty has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Address lyndseyBeil, robehn and sspitsyn CR comments.
>
> Hi Dan,
> It looks good in general.
> But I think moving JVMTI error code checks to Java is a step in a wrong direction.
> First, it makes it inconsistent with all existing JVMTI tests. In this particular case, all the complexity is moved to Java side making it unbalanced. Also, we are working with Leonid toward creating relevant native testing lib for serviceability/jvmti tests (created it for Loom first) which has a support to print symbolic names (for error codes as well) if necessary.
> Not sure, I understood what wrong with the native check_jvmti_status(). It seems the id argument is not needed much and can be removed anyway.
> Thanks,
> Serguei

Hi @sspitsyn,

I rather like the much simpler agent code and it puts the majority of logic
in the .java file so the reader/reviewer doesn't have to jump back and forth
between the two files nearly as much.

The `id` argument had to be passed to the old version of the function calls
in order for the logging messages generated by the agent code to (easily)
have the same thread names as the Java side logging. Once all the error
checking was moved to the Java side, that made the `id` argument no longer
necessary and also allowed the native `check_jvmti_status()` function (along
with other functions) to be removed. So there was nothing wrong with the
native `check_jvmti_status()` function; it was just made obsolete by the code
migration to the Java side.

I consider @robehn's idea here to be good evolution for these kinds of tests to
move code from the native side, where the code is more platform dependent,
to the Java side, where the code is more platform independent.

I hope I have convinced you that this is a good changeset for moving forward.

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

PR: https://git.openjdk.java.net/jdk/pull/2899


More information about the serviceability-dev mailing list