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
-----
