RFR(XL): 8185640: Thread-local handshakes

[Robbin Ehn]

Thank you David for having a look.

I updated after your review, I think I got it all, please see:
http://cr.openjdk.java.net/~rehn/8185640/v9/DavidH-Option-Cleanup-13/webrev/

I'm also updating CSR with product_pd.

Short thing:

On 10/31/2017 11:27 AM, David Holmes wrote:
> 
> I'm also thinking, if this is platform dependent then shouldn't 
> ThreadLocalHandshakes be a product_pd flag, with pd specific default setting - 
> and turning it on when on an unsupported platform should be a error ?

Yes, the error checking already exists in:

  135 Flag::Error ThreadLocalHandshakesConstraintFunc(bool value, bool verbose) {
  136   if (value) {
  137     if (!SafepointMechanism::supports_thread_local_poll()) {
  138       CommandLineError::print(verbose, "ThreadLocalHandshakes not yet 
supported on this platform\n");
  139       return Flag::VIOLATES_CONSTRAINT;
  140     }
  141     if (UseAOT JVMCI_ONLY(|| EnableJVMCI || UseJVMCICompiler)) {
  142       CommandLineError::print(verbose, "ThreadLocalHandshakes not yet 
supported in combination with AOT or JVMCI\n");
  143       return Flag::VIOLATES_CONSTRAINT;
  144     }
  145   }
  146   return Flag::SUCCESS;
  147 }

Sanity tested with handshake benchmark on all supported + 1 unsupported platform.

Thanks, Robbin

> 
> Thanks,
> David
> -----
> 
>> Here is webrev for changes needed:
>> http://cr.openjdk.java.net/~rehn/8185640/v8/Option-Cleanup-12/webrev/
>> And here is CSR:
>> https://bugs.openjdk.java.net/browse/JDK-8189942
>>
>> Manual testing + basic testing done.
>>
>> And since I'm really hoping that this can be the last incremental, here is my 
>> whole patch queue flatten out:
>> http://cr.openjdk.java.net/~rehn/8185640/v8/Full/webrev/
>>
>> Thanks, Robbin
>>
>> On 10/27/2017 04:47 PM, Doerr, Martin wrote:
>>> Hi Robbin,
>>>
>>> excellent. I think this matches what Coleen had proposed, now.
>>> Thanks for doing all the work with so many incremental patches and for 
>>> responding on so many discussions. Seems to be a tough piece of work.
>>>
>>> Best regards,
>>> Martin
>>>
>>>
>>> -----Original Message-----
>>> From: Robbin Ehn [mailto:robbin.ehn at oracle.com]
>>> Sent: Freitag, 27. Oktober 2017 15:15
>>> To: Erik Österlund <erik.osterlund at oracle.com>; Andrew Haley 
>>> <aph at redhat.com>; Doerr, Martin <martin.doerr at sap.com>; Karen Kinnear 
>>> <karen.kinnear at oracle.com>; Coleen Phillimore (coleen.phillimore at oracle.com) 
>>> <coleen.phillimore at oracle.com>
>>> Cc: hotspot-dev developers <hotspot-dev at openjdk.java.net>
>>> Subject: Re: RFR(XL): 8185640: Thread-local handshakes
>>>
>>> Hi all,
>>>
>>> Poll in switches:
>>> http://cr.openjdk.java.net/~rehn/8185640/v7/Interpreter-Poll-Switch-10/
>>>
>>> Poll in return:
>>> http://cr.openjdk.java.net/~rehn/8185640/v7/Interpreter-Poll-Ret-11/
>>>
>>> Please take an extra look at poll in return.
>>>
>>> Sanity tested, big test run still running (99% complete - OK).
>>>
>>> Performance regression for the added polls increased to total of -0.68% vs
>>> global poll. (was -0.44%)
>>>
>>> We are discussing the opt-out option, the newest suggestion is to make it
>>> diagnostic. Opinions?
>>>
>>> For anyone applying these patches, the number 9 patch changes the option from
>>> product. I have not sent that out.
>>>
>>> Thanks, Robbin
>>>
>>>
>>>