Hi David,
Thank you a lot for review!
All suggestions are good. I'll file new bug to cover them.
Thanks,
Serguei
On 4/22/19 17:44, David Holmes wrote:
Hi Serguei,
A belated LGTM - sorry I didn't get to this sooner.
Three minor comments:
1. I would have suggested to add "(Deprecated)" to the description of
the new flag in globals.hpp
2. The new flag should have been added to the deprecated VM options
tests.
3. The new test should run in both a positive and negative mode so
that it also checks that the new flag works.
Thanks for taking care of this - it has been a hard slog. :)
David
-----
On 16/04/2019 7:40 pm, [email protected] wrote:
Please, review the fix of:
https://bugs.openjdk.java.net/browse/JDK-8192936
Webrev (fix from Coleen):
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8192936-redef-add-delete.1/
I've already reviewed and updated the webrev with my suggestions.
Reviewed and approved CSR:
https://bugs.openjdk.java.net/browse/JDK-8221528
Summary:
The fix introduces new VM option
-XX:AllowRedefinitionToAddOrDeleteMethods
for compatibility with previous releases.New option enables old
behavior
and allows the JVM TI RedefineClasses and RetransformClasses to
add/delete
private static and private final instance methods in the new class
versions.
Without this option the old behavior is disabled.
New option is deprecated right away.
The plan is to keep this option for several releases to allow
customers
(tool vendors) to remove dependency on old behavior from their tools.
Testing:
Added new test to verify that class redefinitions which add or
delete methods
return expected JVMTI error codes:
test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/TestAddDeleteMethods.java
Several jvmti, com/sun/jdi and java/lang/instrument tests which
need old behavior are updated to use new flag.
Run locally on Linux-x64 the following test suites in release and
fastdebug mode:
- open/test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/
- vmTestbase_nsk_jvmti
- vmTestbase_nsk_jdi
- vmTestbase_nsk_jdb
- vmTestbase_nsk_jdwp
- jdk_jdi
- jdk_instrument
Submission of corresponding mach5 jobs is in progress.
Thanks,
Serguei
