Okay - seems ready to ship. :)
Thanks,
David
On 22/04/2013 3:20 PM, Rickard Bäckman wrote:
David,
you are right, the only users of this mechanism are the old flatprofiler (which
from what I could figure runs from the WatcherThread) and our sampler mechanism.
That one also runs from the WatcherThread. The point of the assert you are
writing about is to make sure that we actually consumed any post that the
suspended thread
may have issued.
Thanks
/R
On Apr 22, 2013, at 1:53 AM, David Holmes wrote:
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
-----
