[m-rev.] for review: thread-local mutables
Peter Wang
wangp at students.csse.unimelb.edu.au
Fri Jan 12 12:34:00 AEDT 2007
On 2007-01-11, Julien Fischer <juliensf at csse.unimelb.edu.au> wrote:
>
> Hi Peter,
>
> The main issue I have with this design is the interaction of thread
> local mutables with parallel conjunction. The following review comments
> address leave that issue aside.
I've changed it so that the thread-local mutables array is copied at
spawn time, and protected the arrays with mutexes.
> >extras/concurrency/spawn.m:
> > Update the spawn/3 implementation to make child threads inherit the
> > thread-local values of the parent.
> >
> > Make different threads in high-level C grades use different
> > MR_Contexts. This makes it possible to use the same implementation
> > of
> > thread-local mutables as in the low-level C grades.
>
> I think that it's time that spawn/3 (and friends) were moved into the
> standard library.
What's the preferred way to move files with CVS? Also, should they be
contained in a `concurrency' super-module?
> >@@ -812,17 +816,19 @@
> > mutable_trailed :: mutable_trailed,
> > mutable_foreign_names :: maybe(list(foreign_name)),
> > mutable_attach_to_io_state :: bool,
> >- mutable_constant :: bool
> >+ mutable_constant :: bool,
> >+ mutable_thread_local :: bool
> > ).
>
> The value of the mutable_thread_local field should be a specific type,
> like that of the mutable_trailed field, rather than a bool.
Done.
> >+ at item @samp{thread_local}
> >+This attribute causes the mutable to behave differently in threaded
> >+programs. A thread-local mutable can take on a different value in each
> >+thread. When a child thread is spawned, it inherits all the values of
> >+thread-local mutables of the parent thread. Changing the value of a
> >+thread-local mutable does not affect its value in any other threads.
>
> I suggest:
>
> This attribute allows a mutable to take on different values in
> each thread. When ...
Fixed.
> >+#define MR_MAX_THREAD_LOCAL_MUTABLES 128
>
> This restriction can be lifted quite easily, since the compiler (or
> rather mkinit) could potentially have access to the total number of
> thread local mutables in a program.
>
> mkinit and the compiler should be modified to understand a new directive
> that gives the count of the number of thread local mutables in a module.
> When generating the _init.c file mkinit should then compute the total
> and set the value of a global variable in the runtime.
Ok, some other time.
> >Index: runtime/mercury_wrapper.c
> >===================================================================
> >RCS file:
> >/home/mercury/mercury1/repository/mercury/runtime/mercury_wrapper.c,v
> >retrieving revision 1.177
> >diff -u -r1.177 mercury_wrapper.c
> >--- runtime/mercury_wrapper.c 3 Jan 2007 05:17:18 -0000 1.177
> >+++ runtime/mercury_wrapper.c 10 Jan 2007 02:14:36 -0000
...
> >@@ -600,7 +604,7 @@
> > }
> > }
> > #endif /* ! MR_THREAD_SAFE */
> >-#endif /* ! MR_HIGHLEVEL_CODE */
> >+#endif /* ! 0 */
>
> Why has this line changed?
The start of the #ifdef block is:
/*
** XXX The condition here used to be
** #if defined(MR_HIGHLEVEL_CODE) && defined(MR_CONSERVATIVE_GC)
...
*/
#if 0
MR_init_memory();
...
Interdiff follows.
diff -u compiler/make_hlds_passes.m compiler/make_hlds_passes.m
--- compiler/make_hlds_passes.m 9 Jan 2007 01:40:02 -0000
+++ compiler/make_hlds_passes.m 11 Jan 2007 03:27:47 -0000
@@ -1384,8 +1384,8 @@
% Add the foreign_decl and foreign_code items that declare/define
% the global variable used to hold the mutable.
%
-:- pred add_mutable_defn_and_decl(string::in, mer_type::in, bool::in, bool::in,
- prog_context::in, qual_info::in, qual_info::out,
+:- pred add_mutable_defn_and_decl(string::in, mer_type::in, bool::in,
+ mutable_thread_local::in, prog_context::in, qual_info::in, qual_info::out,
module_info::in, module_info::out,
list(error_spec)::in, list(error_spec)::out) is det.
@@ -1484,7 +1484,7 @@
% XXX the second argument should be the name of the mercury predicate,
% with chars escaped as appropriate.
(
- IsThreadLocal = no,
+ IsThreadLocal = mutable_not_thread_local,
LockForeignProcBody = string.append_list([
"#ifdef MR_THREAD_SAFE\n",
" MR_LOCK(&" ++ MutableMutexVarName ++ ",
@@ -1492,7 +1492,7 @@
"#endif\n"
])
;
- IsThreadLocal = yes,
+ IsThreadLocal = mutable_thread_local,
LockForeignProcBody = ""
),
LockForeignProc = pragma_foreign_proc(LockAndUnlockAttrs,
@@ -1511,7 +1511,7 @@
% XXX as above regarding the second argument to MR_UNLOCK.
(
- IsThreadLocal = no,
+ IsThreadLocal = mutable_not_thread_local,
UnlockForeignProcBody = string.append_list([
"#ifdef MR_THREAD_SAFE\n",
" MR_UNLOCK(&" ++ MutableMutexVarName ++ ",
@@ -1519,7 +1519,7 @@
"#endif\n"
])
;
- IsThreadLocal = yes,
+ IsThreadLocal = mutable_thread_local,
UnlockForeignProcBody = ""
),
UnlockForeignProc = pragma_foreign_proc(LockAndUnlockAttrs,
@@ -1540,10 +1540,10 @@
set_thread_safe(proc_thread_safe, UnsafeGetAttrs0, UnsafeGetAttrs),
varset.new_named_var(varset.init, "X", X, ProgVarSet),
(
- IsThreadLocal = no,
+ IsThreadLocal = mutable_not_thread_local,
UnsafeGetCode = "X = " ++ TargetMutableName ++ ";"
;
- IsThreadLocal = yes,
+ IsThreadLocal = mutable_thread_local,
UnsafeGetCode = "MR_get_thread_local_mutable(" ++
TypeName ++ ", X, " ++ TargetMutableName ++ ");"
),
@@ -1591,10 +1591,10 @@
)
),
(
- IsThreadLocal = no,
+ IsThreadLocal = mutable_not_thread_local,
SetCode = TargetMutableName ++ "= X;"
;
- IsThreadLocal = yes,
+ IsThreadLocal = mutable_thread_local,
SetCode = "MR_set_thread_local_mutable(" ++
TypeName ++ ", X, " ++ TargetMutableName ++ ");"
),
@@ -1725,8 +1725,9 @@
% Add the code required to initialise a mutable.
%
-:- pred add_mutable_initialisation(bool::in, bool::in, string::in,
- module_name::in, string::in, prog_varset::in, sym_name::in, prog_term::in,
+:- pred add_mutable_initialisation(bool::in, mutable_thread_local::in,
+ string::in, module_name::in, string::in,
+ prog_varset::in, sym_name::in, prog_term::in,
pragma_foreign_proc_attributes::in, import_status::in, import_status::out,
prog_context::in, module_info::in, module_info::out,
qual_info::in, qual_info::out,
@@ -1757,7 +1758,7 @@
;
IsConstant = no,
(
- IsThreadLocal = no,
+ IsThreadLocal = mutable_not_thread_local,
% Construct the clause for the mutex initialisation predicate.
PreInitCode = string.append_list([
"#ifdef MR_THREAD_SAFE\n",
@@ -1767,7 +1768,7 @@
"#endif\n"
])
;
- IsThreadLocal = yes,
+ IsThreadLocal = mutable_thread_local,
PreInitCode = string.append_list([
TargetMutableName,
" = MR_new_thread_local_mutable_index();\n"
@@ -1814,7 +1815,8 @@
% or not.
%
:- pred get_mutable_global_foreign_decl_defn(module_info::in, mer_type::in,
- string::in, bool::in, bool::in, item::out, item::out) is det.
+ string::in, bool::in, mutable_thread_local::in, item::out, item::out)
+ is det.
get_mutable_global_foreign_decl_defn(ModuleInfo, Type, TargetMutableName,
IsConstant, IsThreadLocal, Decl, Defn) :-
@@ -1824,11 +1826,11 @@
(
Backend = target_c,
(
- IsThreadLocal = no,
+ IsThreadLocal = mutable_not_thread_local,
TypeName = global_foreign_type_name(AlwaysBoxed, lang_c,
ModuleInfo, Type)
;
- IsThreadLocal = yes,
+ IsThreadLocal = mutable_thread_local,
%
% For thread-local mutables, the variable holds an index into an
% array.
@@ -1839,32 +1841,20 @@
% Constant mutables do not require mutexes as their values are never
% updated. Thread-local mutables do not require mutexes either.
%
- (
+ (
( IsConstant = yes
- ; IsThreadLocal = yes
+ ; IsThreadLocal = mutable_thread_local
)
->
- LockDecl = []
+ LockDecl = [],
+ LockDefn = []
;
LockDecl = [
"#ifdef MR_THREAD_SAFE\n",
" extern MercuryLock ",
mutable_mutex_var_name(TargetMutableName), ";\n",
"#endif\n"
- ]
- ),
- DeclBody = string.append_list([
- "extern ", TypeName, " ", TargetMutableName, ";\n" | LockDecl]),
- Decl = item_pragma(compiler(mutable_decl),
- pragma_foreign_decl(lang_c, foreign_decl_is_exported, DeclBody)),
-
- (
- ( IsConstant = yes
- ; IsThreadLocal = yes
- )
- ->
- LockDefn = []
- ;
+ ],
LockDefn = [
"#ifdef MR_THREAD_SAFE\n",
" MercuryLock ",
@@ -1872,6 +1862,12 @@
"#endif\n"
]
),
+
+ DeclBody = string.append_list([
+ "extern ", TypeName, " ", TargetMutableName, ";\n" | LockDecl]),
+ Decl = item_pragma(compiler(mutable_decl),
+ pragma_foreign_decl(lang_c, foreign_decl_is_exported, DeclBody)),
+
DefnBody = string.append_list([
TypeName, " ", TargetMutableName, ";\n" | LockDefn]),
Defn = item_pragma(compiler(mutable_decl),
diff -u compiler/prog_io.m compiler/prog_io.m
--- compiler/prog_io.m 9 Jan 2007 03:08:53 -0000
+++ compiler/prog_io.m 11 Jan 2007 03:33:48 -0000
@@ -1879,7 +1879,7 @@
; mutable_attr_foreign_name(foreign_name)
; mutable_attr_attach_to_io_state(bool)
; mutable_attr_constant(bool)
- ; mutable_attr_thread_local(bool).
+ ; mutable_attr_thread_local(mutable_thread_local).
:- pred parse_mutable_attrs(term::in,
maybe1(mutable_var_attributes)::out) is det.
@@ -1889,10 +1889,12 @@
ConflictingAttributes = [
mutable_attr_trailed(mutable_trailed) -
mutable_attr_trailed(mutable_untrailed),
- mutable_attr_trailed(mutable_trailed) - mutable_attr_thread_local(yes),
+ mutable_attr_trailed(mutable_trailed) -
+ mutable_attr_thread_local(mutable_thread_local),
mutable_attr_constant(yes) - mutable_attr_trailed(mutable_trailed),
mutable_attr_constant(yes) - mutable_attr_attach_to_io_state(yes),
- mutable_attr_constant(yes) - mutable_attr_thread_local(yes)
+ mutable_attr_constant(yes) -
+ mutable_attr_thread_local(mutable_thread_local)
],
(
list_term_to_term_list(MutAttrsTerm, MutAttrTerms),
@@ -1964,7 +1966,7 @@
MutAttr = mutable_attr_constant(yes)
;
String = "thread_local",
- MutAttr = mutable_attr_thread_local(yes)
+ MutAttr = mutable_attr_thread_local(mutable_thread_local)
)
->
MutAttrResult = ok1(MutAttr)
diff -u compiler/prog_item.m compiler/prog_item.m
--- compiler/prog_item.m 7 Jan 2007 06:13:33 -0000
+++ compiler/prog_item.m 11 Jan 2007 03:34:27 -0000
@@ -315,6 +315,12 @@
---> mutable_trailed
; mutable_untrailed.
+ % Indicates if a mutable is thread-local or not.
+ %
+:- type mutable_thread_local
+ ---> mutable_thread_local
+ ; mutable_not_thread_local.
+
% Has the user specified a name for us to use on the target code side
% of the FLI?
%
@@ -340,7 +346,8 @@
= maybe(list(foreign_name)).
:- func mutable_var_constant(mutable_var_attributes) = bool.
:- func mutable_var_attach_to_io_state(mutable_var_attributes) = bool.
-:- func mutable_var_thread_local(mutable_var_attributes) = bool.
+:- func mutable_var_thread_local(mutable_var_attributes)
+ = mutable_thread_local.
:- pred set_mutable_var_trailed(mutable_trailed::in,
mutable_var_attributes::in, mutable_var_attributes::out) is det.
@@ -354,7 +361,7 @@
:- pred set_mutable_var_constant(bool::in,
mutable_var_attributes::in, mutable_var_attributes::out) is det.
-:- pred set_mutable_var_thread_local(bool::in,
+:- pred set_mutable_var_thread_local(mutable_thread_local::in,
mutable_var_attributes::in, mutable_var_attributes::out) is det.
%-----------------------------------------------------------------------------%
@@ -817,11 +824,12 @@
mutable_foreign_names :: maybe(list(foreign_name)),
mutable_attach_to_io_state :: bool,
mutable_constant :: bool,
- mutable_thread_local :: bool
+ mutable_thread_local :: mutable_thread_local
).
default_mutable_attributes =
- mutable_var_attributes(mutable_trailed, no, no, no, no).
+ mutable_var_attributes(mutable_trailed, no, no, no,
+ mutable_not_thread_local).
mutable_var_trailed(MVarAttrs) = MVarAttrs ^ mutable_trailed.
mutable_var_maybe_foreign_names(MVarAttrs) = MVarAttrs ^ mutable_foreign_names.
diff -u extras/concurrency/spawn.m extras/concurrency/spawn.m
--- extras/concurrency/spawn.m 10 Jan 2007 02:22:16 -0000
+++ extras/concurrency/spawn.m 11 Jan 2007 04:24:52 -0000
@@ -66,7 +66,8 @@
/* Store the closure on the top of the new context's stack. */
*(ctxt->MR_ctxt_sp) = Goal;
ctxt->MR_ctxt_next = NULL;
- ctxt->MR_ctxt_thread_local_mutables = MR_THREAD_LOCAL_MUTABLES;
+ ctxt->MR_ctxt_thread_local_mutables =
+ MR_clone_thread_local_mutables(MR_THREAD_LOCAL_MUTABLES);
MR_schedule_context(ctxt);
if (0) {
spawn_call_back_to_mercury_cc_multi:
@@ -153,7 +154,8 @@
*/
args = MR_malloc(sizeof(ME_ThreadWrapperArgs));
args->goal = goal;
- args->thread_local_mutables = MR_THREAD_LOCAL_MUTABLES;
+ args->thread_local_mutables =
+ MR_clone_thread_local_mutables(MR_THREAD_LOCAL_MUTABLES);
if (pthread_create(&thread, MR_THREAD_ATTR, ME_thread_wrapper, args)) {
MR_fatal_error(""Unable to create thread."");
diff -u runtime/mercury_context.h runtime/mercury_context.h
--- runtime/mercury_context.h 10 Jan 2007 02:08:14 -0000
+++ runtime/mercury_context.h 11 Jan 2007 04:18:24 -0000
@@ -205,7 +205,7 @@
MR_Unsigned MR_ctxt_event_number;
#endif
- MR_Word *MR_ctxt_thread_local_mutables;
+ MR_ThreadLocalMuts *MR_ctxt_thread_local_mutables;
};
/*
@@ -229,7 +229,7 @@
MR_Spark *MR_spark_next;
MR_Code *MR_spark_resume;
MR_Word *MR_spark_parent_sp;
- MR_Word *MR_spark_thread_local_mutables;
+ MR_ThreadLocalMuts *MR_spark_thread_local_mutables;
};
#endif
diff -u runtime/mercury_thread.c runtime/mercury_thread.c
--- runtime/mercury_thread.c 10 Jan 2007 02:07:24 -0000
+++ runtime/mercury_thread.c 11 Jan 2007 04:21:01 -0000
@@ -226,0 +227,31 @@
+
+MR_ThreadLocalMuts *
+MR_create_thread_local_mutables(MR_Unsigned numslots)
+{
+ MR_ThreadLocalMuts *muts;
+
+ muts = MR_GC_NEW(MR_ThreadLocalMuts);
+#ifdef MR_THREAD_SAFE
+ pthread_mutex_init(&muts->MR_tlm_lock, MR_MUTEX_ATTR);
+#endif
+ muts->MR_tlm_values = MR_NEW_ARRAY(MR_Word, numslots);
+
+ return muts;
+}
+
+MR_ThreadLocalMuts *
+MR_clone_thread_local_mutables(const MR_ThreadLocalMuts *old_muts)
+{
+ MR_ThreadLocalMuts *new_muts;
+ MR_Unsigned i;
+
+ new_muts = MR_create_thread_local_mutables(MR_num_thread_local_mutables);
+
+ MR_LOCK(&new_muts->MR_tlm_lock, "MR_clone_thread_local_mutables");
+ for (i = 0; i < MR_num_thread_local_mutables; i++) {
+ new_muts->MR_tlm_values[i] = old_muts->MR_tlm_values[i];
+ }
+ MR_UNLOCK(&new_muts->MR_tlm_lock, "MR_clone_thread_local_mutables");
+
+ return new_muts;
+}
diff -u runtime/mercury_thread.h runtime/mercury_thread.h
--- runtime/mercury_thread.h 10 Jan 2007 01:59:18 -0000
+++ runtime/mercury_thread.h 11 Jan 2007 04:30:35 -0000
@@ -201,14 +201,25 @@
/*
** The values of thread-local mutables are stored in an array per Mercury
-** thread. This makes it easy for a newly spawned thread to inherit all the
-** thread-local mutables of its parent thread. The arrays are copied-on-write,
-** but copy-on-spawn would also work.
+** thread. This makes it easy for a newly spawned thread to inherit (copy)
+** all the thread-local mutables of its parent thread.
+** Accesses to the array are protected by a mutex, in case a parallel
+** conjunctions tries to read a thread-local value while another parallel
+** conjunction (in the same Mercury thread) is writing to it.
**
** Each thread-local mutable has an associated index into the array, which is
** allocated to it during initialisation. For ease of implementation there is
** an arbitrary limit to the number of thread-local mutables that are allowed.
*/
+typedef struct MR_ThreadLocalMuts MR_ThreadLocalMuts;
+
+struct MR_ThreadLocalMuts {
+ #ifdef MR_THREAD_SAFE
+ MercuryLock MR_tlm_lock;
+ #endif
+ MR_Word *MR_tlm_values;
+};
+
#define MR_MAX_THREAD_LOCAL_MUTABLES 128
extern MR_Unsigned MR_num_thread_local_mutables;
@@ -218,27 +229,41 @@
extern MR_Unsigned MR_new_thread_local_mutable_index(void);
+/*
+** Allocate a thread-local mutable array.
+*/
+extern MR_ThreadLocalMuts *
+MR_create_thread_local_mutables(MR_Unsigned numslots);
+
+/*
+** Make a copy of a thread-local mutable array.
+*/
+extern MR_ThreadLocalMuts *
+MR_clone_thread_local_mutables(const MR_ThreadLocalMuts *old_muts);
+
#define MR_THREAD_LOCAL_MUTABLES \
(MR_ENGINE(MR_eng_this_context)->MR_ctxt_thread_local_mutables)
-#define MR_SET_THREAD_LOCAL_MUTABLES(mut_array) \
- (MR_THREAD_LOCAL_MUTABLES = mut_array)
+#define MR_SET_THREAD_LOCAL_MUTABLES(tlm) \
+ (MR_THREAD_LOCAL_MUTABLES = tlm)
#define MR_get_thread_local_mutable(type, var, mut_index) \
- (var = *((type *) &MR_THREAD_LOCAL_MUTABLES[mut_index]))
+ do { \
+ MR_ThreadLocalMuts *tlm; \
+ \
+ tlm = MR_THREAD_LOCAL_MUTABLES; \
+ MR_LOCK(&tlm->MR_tlm_lock, "MR_get_thread_local_mutable"); \
+ var = *((type *) &tlm->MR_tlm_values[mut_index]); \
+ MR_UNLOCK(&tlm->MR_tlm_lock, "MR_get_thread_local_mutable"); \
+ } while (0)
#define MR_set_thread_local_mutable(type, var, mut_index) \
do { \
- MR_Word *old_table; \
- MR_Word *new_table; \
- MR_Unsigned i; \
+ MR_ThreadLocalMuts *tlm; \
\
- old_table = MR_THREAD_LOCAL_MUTABLES; \
- new_table = MR_NEW_ARRAY(MR_Word, MR_num_thread_local_mutables); \
- for (i = 0; i < MR_num_thread_local_mutables; i++) { \
- new_table[i] = old_table[i]; \
- } \
- *((type *) &new_table[mut_index]) = var; \
- MR_SET_THREAD_LOCAL_MUTABLES(new_table); \
- } while (0) \
+ tlm = MR_THREAD_LOCAL_MUTABLES; \
+ MR_LOCK(&tlm->MR_tlm_lock, "MR_set_thread_local_mutable"); \
+ *((type *) &tlm->MR_tlm_values[mut_index]) = var; \
+ MR_UNLOCK(&tlm->MR_tlm_lock, "MR_set_thread_local_mutable"); \
+ } while (0)
#endif /* MERCURY_THREAD_H */
diff -u runtime/mercury_wrapper.c runtime/mercury_wrapper.c
--- runtime/mercury_wrapper.c 10 Jan 2007 02:14:36 -0000
+++ runtime/mercury_wrapper.c 11 Jan 2007 04:17:34 -0000
@@ -590,7 +590,7 @@
*/
MR_init_thread(MR_use_now);
MR_SET_THREAD_LOCAL_MUTABLES(
- MR_NEW_ARRAY(MR_Word, MR_MAX_THREAD_LOCAL_MUTABLES));
+ MR_create_thread_local_mutables(MR_MAX_THREAD_LOCAL_MUTABLES));
#ifdef MR_THREAD_SAFE
{
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to: mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions: mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------
More information about the reviews
mailing list