RFR (S): 8029957: PPC64 (part 213): cppInterpreter: memory ordering for object initialization

[Lindenmaier, Goetz]

Hi,

I had a look at the first part of this issue: Whether StoreStore 
is necessary in the interpreter.  Let's for now assume the serialization
page mechanism works on PPC.

In the state transition leaving the VM state, which is executed in the
destructor, ThreadStateTransition::transition() is called, which executes
      if (UseMembar) {
        OrderAccess::fence();
      } else {
        os::write_memory_serialize_page(thread);
      }

os:: write_memory_serialize_page() can not be considered a proper
MemBar, as it only serializes if another thread poisoned the page.
Thus it does not qualify to order the initialization and the publishing
of the object.

You are right, if UseMembar is true, the StoreStore in the interpreter
is superfluous.  We could guard the StoreStores in the interpreter by
!UseMembar.

But then again, one is to order the publishing of the thread states, 
the other to enforce some Java semantics.  I don't know whether everybody
who changes in one place is aware of both issues.  But if you want to,
I'll add a !UseMembar in the interpreter.  Maybe it would be a good idea
to document the double use in interfaceSupport.cpp, too.  And maybe 
add an assertion of some kind.

We're digging into the other issue currenty, whether the serialization page
works on ppc.  We understand your concerns and have no simple answer to 
it right now.  At least, in our VM and in the port there are no known problems
with the state transitions.

Best regards,
  Goetz.



-----Original Message-----
From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com] 
Sent: Donnerstag, 16. Januar 2014 19:16
To: David Holmes; Lindenmaier, Goetz
Cc: 'ppc-aix-port-dev at openjdk.java.net'; 'hotspot-dev Source Developers'
Subject: Re: RFR (S): 8029957: PPC64 (part 213): cppInterpreter: memory ordering for object initialization

Changes are in C++ Interpreter so it does not affect Oracle VM.
But David has point here. I would like to hear the explanation too.

BTW, I see that for ppc64:

src/cpu/ppc/vm//globals_ppc.hpp:define_pd_global(bool, UseMembar, false);

as result write_memory_serialize_page() is used in 
ThreadStateTransition::transition().

Is it not enough on PPC64?

Thanks,
Vladimir

On 1/15/14 9:30 PM, David Holmes wrote:
> Can I get some response on this please - specifically the redundancy wrt
> IRT_ENTRY actions.
>
> Thanks,
> David
>
> On 20/12/2013 11:55 AM, David Holmes wrote:
>> Still catching up ...
>>
>> On 11/12/2013 9:46 PM, Lindenmaier, Goetz wrote:
>>> Hi,
>>>
>>> this change adds StoreStore barriers after object initialization and
>>> after constructor calls in the C++ interpreter.  This assures no
>>> uninitialized
>>> objects or final fields are visible.
>>> http://cr.openjdk.java.net/~goetz/webrevs/8029957-0-moci/
>>
>> The InterpreterRuntime calls are all IRT_ENTRY points which will utilize
>> thread state transitions that already include a full "fence" so the
>> storestore barriers are redundant in those cases.
>>
>> The fastpath _new storestore seems okay.
>>
>> I don't know how handle_return gets used to know if it is reasonable or
>> not.
>>
>> I was trying, unsuccessfully, to examine the same code in the
>> templateInterpreter to see how it handles these cases as it naturally
>> has the same object-initialization-safety requirements (though these can
>> be handled in a number of different ways other than an unconditional
>> storestore barrier at the end of the initialization and construction
>> phases.
>>
>> David
>> -----
>>
>>> Please review and test this change.
>>>
>>> Best regards,
>>>    Goetz.
>>>