RFR 8202201: All oop stores in the x64 interpreter are treated as volatile when using G1
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Mon Sep 17 12:36:28 UTC 2018
On 9/17/18 8:34 AM, Erik Österlund wrote:
> Hi Coleen,
>
> On 2018-09-17 14:02, coleen.phillimore at oracle.com wrote:
>>
>>
>> On 9/17/18 12:55 AM, David Holmes wrote:
>>> Hi Coleen,
>>>
>>> Wow. Have you run any performance tests to see what we've gained by
>>> eliding all the unnecessary "lock addl" psuedo-mfence?
>>
>> No. I would be surprised if there's any noticable difference except
>> in microbenchmarks.
>
> I did some Xint measurements when I accidentally found out we had this
> problem, and didn't notice any difference, sadly. I was hoping for
> some speedup. The whole thing made me giggle nevertheless.
>
>>>
>>> On 15/09/2018 1:44 AM, coleen.phillimore at oracle.com wrote:
>>>> Summary: ran out of registers, generated volatile and non-volatile
>>>> branches.
>>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8202201.01/webrev
>>>
>>> Generally looks good. Your webrev seems to have picked up some noise
>>> from Mikael's unused Label changes ??
>>
>> I manually made the same changes myself since I'm going to have to
>> merge when he checks his in.
>>
>>>
>>> One query. Can you comment on the change to the 32-bit code where we
>>> no longer use MO_RELAXED for the store? I'm assuming this is just a
>>> potential optimization in the case that the store might be ordered
>>> somehow by default. Though why this would be 32-bit specific I don't
>>> know.
>>
>> Yeah, it looks like an optimization. Maybe Erik can tell us why he
>> added it. I assume he added it.
>
> MO_RELAXED is default, so you don't need to add it explicitly.
>
> Also, changes look good to me. Not sure about the unused labels, but
> if you say there is a plan for those, then I guess I am okay with that.
Thanks Erik. I manually removed the unused labels that Mikael had so
that I could merge my changes more easily, which seems to have worked.
Coleen
>
> Thanks,
> /Erik
>
>> Thanks,
>> Coleen
>>
>>>
>>> Thanks,
>>> David
>>>
>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8202201
>>>>
>>>> Tested with applications/jcstress, tier1&2 and jvmti tests
>>>> (sometimes they filled up interpreter fixed space).
>>>>
>>>> putfield bytecode size is 2176 bytes now, 1248 bytes before
>>>> putstatic bytecode size is 1344 bytes now, 864 before
>>>> fast_xputfield is same 96 bytes.
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>
>
More information about the hotspot-runtime-dev
mailing list