RFR(xs): 8202073: MetaspaceAllocationTest gtest shall lock during space creation

Thomas Stüfe thomas.stuefe at gmail.com
Mon Apr 23 16:40:08 UTC 2018


Hi Axel,

On Mon, Apr 23, 2018 at 5:27 PM, Siebenborn, Axel
<axel.siebenborn at sap.com> wrote:
> Hi Thomas,
>
> Is it really necessary, that the metadata_lock is acquired for the initialization of ClassLoaderMetaspace?

I think so :)

> For all current uses of the ClassLoaderMetaspace constructor the lock is hold anyway.
> But the Constructor could be used somewhere else and in that case, I would not want the assertion in add_chunk.

I do not see how SpaceManager::add_chunk() could be used concurrently.
It modifies SpaceManager state. So it needs to be locked. Which all
current call paths - including the creation path - do. I just make
this requirement explicit.

I guess in theory one could live without lock at creation time. Not
sure it is worth the added complexity, though.

> But I'm not that in the code, maybe I'm missing something.
>
> Besides that, the change looks good.
> However, I'm not a reviewer.
>

Thanks. I would have added your name as reviewer anyway, but I already
pushed, sorry!

Best Regards, Thomas


> Cheers,
> Axel
>
>> -----Original Message-----
>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>> bounces at openjdk.java.net] On Behalf Of Thomas Stüfe
>> Sent: Samstag, 21. April 2018 07:36
>> To: Hotspot dev runtime <hotspot-runtime-dev at openjdk.java.net>
>> Subject: Re: RFR(xs): 8202073: MetaspaceAllocationTest gtest shall lock
>> during space creation
>>
>> Hi all,
>>
>> may I have a second reviewer please.
>>
>> new webrev (I missed an include and broke the non-pch builds):
>>
>> http://cr.openjdk.java.net/~stuefe/webrevs/8202073-
>> MetaspaceAllocationTest-gtest-shall-lock-during-space-
>> creation/webrev.01/webrev/
>>
>> Thanks, Thomas
>>
>>
>> On Fri, Apr 20, 2018 at 9:11 AM, Thomas Stüfe <thomas.stuefe at gmail.com>
>> wrote:
>> > May I please have reviews for this small fix to the
>> > metaspace-allocate-stresstest-gtest:
>> >
>> > Bug: https://bugs.openjdk.java.net/browse/JDK-8202073
>> >
>> > webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8202073-
>> MetaspaceAllocationTest-gtest-shall-lock-during-space-
>> creation/webrev.00/webrev/
>> >
>> > MetaspaceAllocationTest mimicks ClassLoaderMetaspace creation,
>> > allocation and destruction to stress metaspace allocation.
>> >
>> > So it runs outside the normal create-allocate-delete cycle in the VM.
>> > For instance, there are no associated ClassLoaderData. So, the test
>> > needs to mimick the surroundings of the VM.
>> >
>> > This means, when creating ClassLoaderMetaspace objects, the test needs
>> > to lock its associated lock. In the VM, this happens in
>> > ClassLoaderData::metaspace_non_null(), and there the associated lock
>> > is pulled before allocation too.
>> >
>> > Thanks, Thomas


More information about the hotspot-runtime-dev mailing list