RFR: 8011882: Replace spin loops as back off when suspending

[David Holmes]

Hi Rickard,

On 20/04/2013 12:19 AM, Rickard Bäckman wrote:
> David,
>
> here is an updated webrev with the changes incorporated.
>
> http://cr.openjdk.java.net/~rbackman/8011882.1/

The changes look reasonable.

My only concern is the:

assert(!sr_semaphore.trywait(), "semaphore has invalid state");

I'm not completely clear on the higher-level protocols and usage of this 
API and what actions can be attempted concurrently. These asserts 
indicate a strict one thread at a time usage - is that right?

The validation of all this comes in the testing of course.

Thanks,
David

> Thanks
> /R
>
> On Apr 16, 2013, at 9:44 AM, Rickard Bäckman wrote:
>
>> David,
>>
>> thanks for the input, I'll go back to the split versions and update the timing.
>>
>> /R
>>
>> On Apr 16, 2013, at 1:27 AM, David Holmes wrote:
>>
>>> PS. Also see the existing unpackTime and compute_abstime helper functions for dealing with pthread/POSIX absolute timed-waits. Better than using javaTimeMillis()
>>>
>>> David
>>>
>>> On 15/04/2013 10:50 PM, David Holmes wrote:
>>>> On 15/04/2013 10:07 PM, Rickard Bäckman wrote:
>>>>> David,
>>>>>
>>>>> this is what the suggested semaphore.cpp/semaphore.hpp. Is that what
>>>>> you are looking for?
>>>>
>>>> <sigh> I thought so till I saw it - far uglier and complicated than I
>>>> had hoped. Sadly the three separate versions wins for me.
>>>>
>>>> By the way you can't do this:
>>>>
>>>> 116 bool Semaphore::timedwait(unsigned int sec, int nsec) {
>>>> 117   struct timespec ts;
>>>> 118   jlong endtime = os::javaTimeNanos() + (sec * NANOSECS_PER_SEC) +
>>>> nsec;
>>>> 119   ts.tv_sec = endtime / NANOSECS_PER_SEC;
>>>> 120   ts.tv_nsec = endtime % NANOSECS_PER_SEC;
>>>>
>>>> javaTimeNanos is not wall-clock time, but the POSIX sem_timewait
>>>> requires an absolute time - you need to use javaTimeMillis(). Which also
>>>> means the wait will be affected by changes to wall-clock time.
>>>>
>>>> David
>>>> -----
>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~rbackman/webrev/
>>>>>
>>>>> Thanks
>>>>> /R
>>>>>
>>>>> On Apr 15, 2013, at 8:59 AM, David Holmes wrote:
>>>>>
>>>>>> On 15/04/2013 4:55 PM, Rickard Bäckman wrote:
>>>>>>> David,
>>>>>>>
>>>>>>> any new thoughts?
>>>>>>
>>>>>> Not a new one but I think factoring into Semaphore.hpp/cpp and using
>>>>>> a few ifdefs is better than three versions of the Semaphore class.
>>>>>> The signal thread could use it also.
>>>>>>
>>>>>> David
>>>>>>
>>>>>>> Thanks
>>>>>>> /R
>>>>>>>
>>>>>>> On Apr 12, 2013, at 8:06 AM, Rickard Bäckman wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> On Apr 12, 2013, at 7:34 AM, David Holmes wrote:
>>>>>>>>
>>>>>>>>> On 12/04/2013 3:01 PM, Rickard Bäckman wrote:
>>>>>>>>>>
>>>>>>>>>> On Apr 12, 2013, at 1:04 AM, David Holmes wrote:
>>>>>>>>>>
>>>>>>>>>>> On 11/04/2013 11:02 PM, Rickard Bäckman wrote:
>>>>>>>>>>>> On Apr 11, 2013, at 2:39 PM, David Holmes wrote:
>>>>>>>>>>>>> So what did you mean about pthread_semaphore (what is that
>>>>>>>>>>>>> anyway?) ??
>>>>>>>>>>>>
>>>>>>>>>>>> Never mind, pthread condition variables.
>>>>>>>>>>>
>>>>>>>>>>> Ah I see.
>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> I really, really, really don't like seeing three versions of
>>>>>>>>>>>>> this class :( Can't BSD and Linux at least share a POSIX
>>>>>>>>>>>>> version? (And I wonder if we can actually mix-n-match UI
>>>>>>>>>>>>> threads on Solaris with POSIX semaphores on Solaris?)
>>>>>>>>>>>>
>>>>>>>>>>>> I don't like it either, our OS code isn't really helpful when
>>>>>>>>>>>> it comes do reusing things :) Not sure how I would layout
>>>>>>>>>>>> things to make them only available on BSD (Not Mac) and Linux.
>>>>>>>>>>>> I guess os_posix.hpp with lots of #ifdefs, but I'm not sure I"m
>>>>>>>>>>>> feeling that happy about that.
>>>>>>>>>>>
>>>>>>>>>>> Why would the os_posix version need a lot of ifdefs?
>>>>>>>>>>
>>>>>>>>>> Well, I guess we would need:
>>>>>>>>>>
>>>>>>>>>> (in ifdef pseudo language)
>>>>>>>>>>
>>>>>>>>>> #ifdef (LINUX || (BSD && !APPLE))
>>>>>>>>>> …
>>>>>>>>>> #endif
>>>>>>>>>
>>>>>>>>> But if it isn't "posix" then we won't be building os_posix - right?
>>>>>>>>
>>>>>>>> Linux, Solaris, Bsd & Mac builds and include os_posix. They are all
>>>>>>>> "implementing posix" we are just not using the same semaphore
>>>>>>>> implementation on all of them.
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> The second interesting problem this will get us into is that
>>>>>>>>>> sem_t is not declared in this context. Where do we put the
>>>>>>>>>> #include <semaphore.h>? Impossible in os_posix.hpp since it is
>>>>>>>>>> included in the middle of a class definition. I could put it in
>>>>>>>>>> os.hpp in the #ifdef path that does the jvm_platform.h includes,
>>>>>>>>>> not sure if that is very pretty either.
>>>>>>>>>
>>>>>>>>> Semaphores are already used by the signal handler thread -
>>>>>>>>> semaphore.h is included in os_linux.cpp etc, so why would os_posix
>>>>>>>>> be any different ?
>>>>>>>>>
>>>>>>>>> But couldn't we just have a Semaphore.h/cpp with any needed ifdefs?
>>>>>>>>>
>>>>>>>>>>> Do we really have four versions:
>>>>>>>>>>> - linux (posix)
>>>>>>>>>>> - BSD (posix)
>>>>>>>>>>> - Solaris
>>>>>>>>>>> - Mac (different to BSD?)
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 3:
>>>>>>>>>> 1) linux & bsd uses the sem_ interface
>>>>>>>>>> 2) solaris uses the sema_ interface
>>>>>>>>>> 3) mac uses the semaphore_ interface
>>>>>>>>>
>>>>>>>>> Okay but if mac is BSD why can't we use bsd ie posix interface
>>>>>>>>> instead of the mach semaphore_ ?
>>>>>>>>
>>>>>>>> Because apple decided not to implement sem_timedwait.
>>>>>>>> On Solaris we use sema_ because sem_ requires us to link with -lrt
>>>>>>>> which we currently don't (and I'm not really feeling like adding it)
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> BTW I like the idea of using the semaphore, we're just haggling on
>>>>>>>>> the details. ;-)
>>>>>>>>
>>>>>>>> I'm fine with that :)
>>>>>>>>
>>>>>>>> /R
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>>> /R
>>>>>>>>>>
>>>>>>>>>>> ??
>>>>>>>>>>>
>>>>>>>>>>> David
>>>>>>>>>>> -----
>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>
>