RFR(S) 8249276 CDS archived objects must have "neutral" markwords
Coleen Phillimore
coleen.phillimore at oracle.com
Tue Aug 11 16:09:27 UTC 2020
http://cr.openjdk.java.net/~iklam/jdk16/8249276-reset-archive-obj-headers.v03/test/hotspot/jtreg/runtime/cds/appcds/javaldr/LockDuringDumpAgent.mf.html
This doesn't have a copyright. There are tests like
test/hotspot/jtreg/serviceability/jvmti/GetObjectSizeClass.java that
create the manifest file inside the .java source code so you don't need
this file, which I think you can do here:
http://cr.openjdk.java.net/~iklam/jdk16/8249276-reset-archive-obj-headers.v03/test/hotspot/jtreg/runtime/cds/appcds/javaldr/LockDuringDump.java.html
Coleen
On 8/11/20 1:29 AM, Ioi Lam wrote:
>
>
> On 8/6/20 5:55 PM, David Holmes wrote:
>> Hi Ioi,
>>
>> On 6/08/2020 8:26 am, Ioi Lam wrote:
>>> Hi David & Coleen,
>>>
>>> Thanks for the review. I have created a new version:
>>>
>>> http://cr.openjdk.java.net/~iklam/jdk16/8249276-reset-archive-obj-headers.v02/
>>>
>>
>> Functional changes are good.
>>
>> For the test, as per our offline chat, the Helper class is not being
>> archived. This line:
>>
>> 66 TestCommon.testDump(appJar,
>> TestCommon.list(LockDuringDumpApp.class.getName()),
>>
>> should be
>>
>> 66 TestCommon.testDump(appJar,
>> TestCommon.list(appClasses),
>>
>>
>
> Hi David,
>
> (The Helper class was actually archived, because it was loaded by
> LockDuringDumpAgent during dump time, so it was not necessary to
> explicitly include it in the class list. ....)
>
> Anyway, it wasn't clear to me whether any code (now or in the future)
> in the JVM might synchronize on the java mirrors. So I've updated the
> test case to synchronize on a literal string instead. This way I know
> that only the test, and no one else, will synchronize on it.
>
> I also added comments to explain how the test works.
>
> http://cr.openjdk.java.net/~iklam/jdk16/8249276-reset-archive-obj-headers.v03/
>
>
> Thanks
> - Ioi
>
>
>>>
>>> I created a new test case (LockDuringDump), and incorporated David's
>>> fix for JDK-8250606 [1] during my testing:
>>>
>>> + Without my patch, LockDuringDumpApp would crash at runtime, because
>>> we have an invalid markword for the Helper.class object.
>>>
>>> + Without the JDK-8250606 patch, we would assert during CDS dump time.
>>>
>>> I also removed the assert in HeapShared::oop_hash() since it's
>>> irrelevant after JDK-8250606.
>>>
>>> I intend to push my patch after JDK-8250606 is integrated.
>>>
>>>
>>> -------
>>>
>>> More comments below
>>>
>>> On 8/3/20 6:13 PM, David Holmes wrote:
>>>> Hi Ioi,
>>>>
>>>> On 27/07/2020 3:20 pm, Ioi Lam wrote:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8249276
>>>>> http://cr.openjdk.java.net/~iklam/jdk16/8249276-reset-archive-obj-headers.v01/
>>>>>
>>>>>
>>>>> Please review this change (initial patch provided by David Holmes;
>>>>> I rearranged it a little and added more comments).
>>>>>
>>>>> We reinitialize the markWord of all archived objects to remove any
>>>>> side effect of GC/locking/etc during "java -Xshare:dump".
>>>>
>>>> Changes look good to me (naturally :) ). A couple of nits:
>>>>
>>>> int hash_archived = archived_oop->identity_hash();
>>>> assert(hash_original == hash_archived, "Different hash codes:
>>>> original %x, archived %x", hash_original, hash_archived);
>>>>
>>>> the has_archived setting should be in ifdef DEBUG.
>>>>
>>>
>>> Fixed
>>>
>>>
>>>> 42 Object.class.wait();
>>>> 43 }
>>>> 44 System.out.println("Huh?? notified??");
>>>>
>>>> That could be a spurious wakeup. You could just use Thread.sleep
>>>> with a suitably long sleep time. that said I'm not sure how the
>>>> test is intended to operate. If premain returns after starting the
>>>> thread there is no guarantee the thread will start executing and
>>>> lock the object before the dump actually commences. Even the
>>>> sighting of "Let's wait ....." is not sufficient to guarantee that
>>>> unless you move it inside the sync block.
>>>>
>>>
>>> I have fixed the test case so that the "Let's hold the lock" message
>>> is printed after the lock is held, and the lock is never released
>>> afterwards.
>>>
>>> I also added synchronization to make sure the premain() method
>>> doesn't return until the child thread has held the lock. The log
>>> file shows that theses messages are printed before CDS tries to load
>>> classes from the classlist:
>>>
>>> [0.005s][info][cds] Allocated shared space: 3221225472 bytes at
>>> 0x0000000800000000
>>> inside LockDuringDumpAgent:
>>> jdk.internal.loader.ClassLoaders$AppClassLoader at 707f7052
>>> Helper class is initialized
>>> Class = class LockDuringDumpApp$Helper
>>> Let's hold the lock on Helper.class forever .....
>>> Thread has started
>>> [0.145s][info][cds] Loading classes to share ...
>>>
>>>>> (David mentioned in the bug comments about assertions inside the
>>>>> identity_hash() call, but maybe this should be fixed in a
>>>>> different bug?)
>>>>
>>>> Yes those assertions are being removed under:
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8250606
>>>>
>>>
>>> Thanks
>>> - Ioi
>>>
>>> -------
>>>
>>> [1]
>>> https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-August/040985.html
>>>
>>>
>>>
>
More information about the hotspot-runtime-dev
mailing list