On Sat, 10 Sep 2022 14:39:54 GMT, Daniel D. Daugherty <dcu...@openjdk.org>
wrote:
>> Alex Menkov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> updated test. comments, code reorg
>
> test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineRetransform/RedefineRetransform.java
> line 240:
>
>> 238: case 5:
>> 239: test("Redefine-Retransform-Redefine-Retransform with CFLH",
>> () -> {
>> 240: redefine(1, 5); // CFLH sets cached class bytes to
>> ver 1
>
> I'm having trouble understanding why the CFLH version is '5' here.
> Update: I _think_ this is just to have the CFLH return a different version
> of the class bytes before the RedefineClasses() call does its work. I
> don't understand why you want to do this...
Test cases 1-4 are from the bug description.
I added test cases 5 & 6 to verify additional code paths - they are the same as
3 & 4, but in RedefineClasses we provide new class bytes in CFLH.
I.e. in cases 3 and 4 after RedefineClasses classes have no cached bytes and
class bytes are reconstituted in the subsequent Retransform;
In case 5 and 6 cache_bytes buffer is created during RedefineClasses,
RetransformClasses use existing cache.
> test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineRetransform/RedefineRetransform.java
> line 242:
>
>> 240: redefine(1, 5); // CFLH sets cached class bytes to
>> ver 1
>> 241: retransform(2, 1); // uses existing cache
>> 242: redefine(3, 6); // resets cached class bytes,
>
> I'm having trouble understanding why the CFLH version is '6' here.
> Update: I _think_ this is just to have the CFLH return a different version
> of the class bytes before the RedefineClasses() call does its work. I
> don't understand why you want to do this...
>
> Perhaps:
> ```// resets cached class bytes to nullptr,```
Fixed the comment
> test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineRetransform/RedefineRetransform.java
> line 251:
>
>> 249: test("Retransform-Redefine-Retransform-Retransform with
>> CFLH", () -> {
>> 250: retransform(1, 0); // sets cached class bytes to ver 0
>> (initially loaded)
>> 251: redefine(2, 5); // resets cached class bytes,
>
> I'm having trouble understanding why the CFLH version is '5' here.
> Update: I _think_ this is just to have the CFLH return a different version
> of the class bytes before the RedefineClasses() call does its work. I
> don't understand why you want to do this...
>
> Perhaps
> ```// resets cached class bytes to nullptr,```
Fixed the comment
-------------
PR: https://git.openjdk.org/jdk/pull/10032
