David, here is an updated webrev with the changes incorporated.
http://cr.openjdk.java.net/~rbackman/8011882.1/ 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 >>>>>>>>>> ----- >>>>>>>>> >>>>>>> >>>>>> >>>> >
