[m-rev.] diff: Make wait_future not need to take a lock in many cases.
Paul Bone
pbone at csse.unimelb.edu.au
Tue May 10 10:27:48 AEST 2011
When a program 'waits' on a future it takes the future's lock, checks if the
future is available and if it is reads the value and unlocks the future. We
can avoid the locking operation in many cases by testing if the future is
available before taking the lock. If the future is not available then take
the lock and re-test to see if the future is available.
To make this safe we now write the future's value before writing to the field
that says it's available, these two writes are stored in the correct order by
using an 'sfence' instruction.
runtime/mercury_par_builtin.m:
As above.
Also re-order the fields of the future structure, putting fut_value and
fut_signalled next to each other, they're more likely to be in te same
cache line this way.
library/Mmakefile:
Make par_builtin.o depend on mercury_par_builtin.h in the runtime.
Index: library/Mmakefile
===================================================================
RCS file: /home/mercury1/repository/mercury/library/Mmakefile,v
retrieving revision 1.178
diff -u -p -b -r1.178 Mmakefile
--- library/Mmakefile 31 Jan 2011 15:21:13 -0000 1.178
+++ library/Mmakefile 10 May 2011 00:09:07 -0000
@@ -477,6 +477,7 @@ $(os_subdir)table_builtin.pic_o \
$(os_subdir)par_builtin.$O \
$(os_subdir)par_builtin.pic_o \
: ../runtime/mercury_context.h \
+ ../runtime/mercury_par_builtin.h \
../runtime/mercury_thread.h
#-----------------------------------------------------------------------------#
Index: runtime/mercury_par_builtin.h
===================================================================
RCS file: /home/mercury1/repository/mercury/runtime/mercury_par_builtin.h,v
retrieving revision 1.4
diff -u -p -b -r1.4 mercury_par_builtin.h
--- runtime/mercury_par_builtin.h 13 Apr 2011 13:19:41 -0000 1.4
+++ runtime/mercury_par_builtin.h 10 May 2011 00:21:49 -0000
@@ -21,6 +21,7 @@ vim: ft=c ts=4 sw=4 et
#include "mercury_context.h"
#include "mercury_thread.h"
#include "mercury_threadscope.h"
+#include "mercury_atomic_ops.h"
#ifdef MR_THREAD_SAFE
@@ -28,12 +29,12 @@ vim: ft=c ts=4 sw=4 et
/* lock preventing concurrent accesses */
MercuryLock MR_fut_lock;
- /* whether this future has been signalled yet */
- int MR_fut_signalled;
-
/* linked list of all the contexts blocked on this future */
MR_Context *MR_fut_suspended;
+ /* whether this future has been signalled yet */
+ volatile int MR_fut_signalled;
+
MR_Word MR_fut_value;
};
@@ -89,47 +90,49 @@ vim: ft=c ts=4 sw=4 et
} while (0)
/*
- ** It would be nice if we could rely on an invariant such as
- ** `if MR_fut_signalled is true, then reading MR_fut_value is ok'
- ** even *without* wrapping up those two field accesses in the mutex,
- ** taking the mutex only when MR_fut_signalled is false. (We would
- ** then have to repeat the test of MR_fut_signalled, of course.)
- ** Unfortunately, memory systems today cannot be relied on to provide
- ** the required level of consistency; some don't have any way to ask
- ** for the necessary memory barrier.
+ ** If MR_fut_signalled is true then we guarantee that reading MR_fut_value
+ ** is safe, even without a lock, see the corresponding code in
+ ** MR_par_builtin_signal_future();
+ ** If MR_fut_signalled is false then we do take a lock and re-read the
+ ** this value (to ensure there was not a race).
*/
MR_declare_entry(mercury__par_builtin__wait_resume);
#define MR_par_builtin_wait_future(Future, Value) \
do { \
- MR_LOCK(&(Future->MR_fut_lock), "future.wait"); \
- \
if (Future->MR_fut_signalled) { \
Value = Future->MR_fut_value; \
+ MR_maybe_post_wait_future_nosuspend(Future); \
+ } else { \
+ MR_LOCK(&(Future->MR_fut_lock), "future.wait"); \
+ MR_CPU_LFENCE; \
+ if (Future->MR_fut_signalled) { \
MR_UNLOCK(&(Future->MR_fut_lock), "future.wait"); \
+ Value = Future->MR_fut_value; \
MR_maybe_post_wait_future_nosuspend(Future); \
} else { \
MR_Context *ctxt; \
\
/* \
- ** Put the address of the future at a fixed place known to \
- ** mercury__par_builtin__wait_resume, to wit, the top of \
- ** the stack. \
+ ** Put the address of the future at a fixed place \
+ ** known to mercury__par_builtin__wait_resume, to wit, \
+ ** the top of the stack. \
*/ \
MR_incr_sp(1); \
MR_sv(1) = (MR_Word) Future; \
\
/* \
- ** Save this context and put it on the list of suspended \
- ** contexts for this future. \
+ ** Save this context and put it on the list of \
+ ** suspended contexts for this future. \
*/ \
ctxt = MR_ENGINE(MR_eng_this_context); \
MR_save_context(ctxt); \
\
ctxt->MR_ctxt_resume = \
MR_ENTRY(mercury__par_builtin__wait_resume); \
- ctxt->MR_ctxt_resume_owner_engine = MR_ENGINE(MR_eng_id); \
+ ctxt->MR_ctxt_resume_owner_engine = \
+ MR_ENGINE(MR_eng_id); \
ctxt->MR_ctxt_next = Future->MR_fut_suspended; \
Future->MR_fut_suspended = ctxt; \
\
@@ -139,11 +142,12 @@ vim: ft=c ts=4 sw=4 et
MR_maybe_post_stop_context; \
MR_ENGINE(MR_eng_this_context) = NULL; \
/* \
- ** MR_idle will try to run a different context as that has \
- ** good chance of unblocking the future. \
+ ** MR_idle will try to run a different context as that \
+ ** has good chance of unblocking the future. \
*/ \
MR_idle(); \
} \
+ } \
} while (0)
#define MR_par_builtin_get_future(Future, Value) \
@@ -171,8 +175,13 @@ vim: ft=c ts=4 sw=4 et
if (Future->MR_fut_signalled) { \
assert(Future->MR_fut_value == Value); \
} else { \
- Future->MR_fut_signalled = MR_TRUE; \
Future->MR_fut_value = Value; \
+ /* \
+ ** Ensure that the value is available before we update \
+ ** MR_fut_signalled. \
+ */ \
+ MR_CPU_SFENCE; \
+ Future->MR_fut_signalled = MR_TRUE; \
} \
\
/* Schedule all the contexts blocked on this future. */ \
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 489 bytes
Desc: Digital signature
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20110510/e669c7d6/attachment.sig>
More information about the reviews
mailing list