[m-rev.] for review: thread-local mutables

[Julien Fischer]

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.

On Wed, 10 Jan 2007, Peter Wang wrote:

> Estimated hours taken: 15
> Branches: main
>
> Add support for thread-local mutables.  These can take on a different value for
> each Mercury thread.  Child threads automatically inherit the thread-local
> values of the parent thread that spawned it.
>
> compiler/make_hlds_passes.m:
> compiler/prog_io.m:
> compiler/prog_item.m:
> compiler/prog_mutable.m:
> 	Accept a `thread_local' attribute for mutables and update the
> 	source-to-source transformation.
>
> doc/reference_manual.texi:
> 	Document the `thread_local' attribute as a Melbourne Mercury compiler
> 	extension.
>
> runtime/mercury_context.c:
> runtime/mercury_context.h:
> 	Add a `thread_local_mutables' field to MR_Context, which points to an
> 	array which holds all the values of thread-local mutables in the program.
> 	Each thread-local mutable has an associated index into the array, which
> 	is allocated during initialisation.  The arrays are copied-on-write.  A
> 	child thread inherits the parent's thread-locals simply by pointing to
> 	the same array.
>
> 	Add a `thread_local_mutables' field to MR_Spark and update the parallel
> 	conjunction implementation to take into account thread-locals.
> 	Note that setting the value of a thread-local mutable inside a parallel
> 	conjunction is not supported.
>
> runtime/mercury_thread.c:
> runtime/mercury_thread.h:
> 	Add the functions and macros which are used by the code generated for
> 	thread-local mutables.
>
> runtime/mercury_wrapper.c:
> 	Allocate a thread-local mutable array for the initial context at
> 	startup.
>
> 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.

> tests/hard_coded/mutable_decl.exp:
> tests/hard_coded/mutable_decl.m:
> tests/hard_coded/pure_mutable.exp:
> tests/hard_coded/pure_mutable.m:
> tests/invalid/bad_mutable.err_exp:
> tests/invalid/bad_mutable.m:
> 	Add some thread-local mutables to these test cases.

...

> Index: compiler/prog_item.m
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/compiler/prog_item.m,v
> retrieving revision 1.24
> diff -u -r1.24 prog_item.m
> --- compiler/prog_item.m	6 Jan 2007 09:23:48 -0000	1.24
> +++ compiler/prog_item.m	7 Jan 2007 06:13:33 -0000
> @@ -340,6 +340,7 @@
>     = 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.
>
> :- pred set_mutable_var_trailed(mutable_trailed::in,
>     mutable_var_attributes::in, mutable_var_attributes::out) is det.
> @@ -353,6 +354,9 @@
> :- 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,
> +    mutable_var_attributes::in, mutable_var_attributes::out) is det.
> +
> %-----------------------------------------------------------------------------%
> %
> % Pragmas
> @@ -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.

...

> Index: doc/reference_manual.texi
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/doc/reference_manual.texi,v
> retrieving revision 1.377
> diff -u -r1.377 reference_manual.texi
> --- doc/reference_manual.texi	6 Jan 2007 10:49:04 -0000	1.377
> +++ doc/reference_manual.texi	9 Jan 2007 03:12:58 -0000
> @@ -4837,6 +4837,19 @@
> It also cannot be specified together with an explicit @samp{trailed}
> attribute.
>
> + 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 ...

> +The @samp{thread_local} attribute cannot be specified together with
> +either of the @samp{trailed} or @samp{constant} attributes.
> +
> + at c Note: don't set thread-local mutables in parallel conjunctions, as
> + at c the behaviour will be unpredictable.

...

> Index: runtime/mercury_thread.h
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_thread.h,v
> retrieving revision 1.19
> diff -u -r1.19 mercury_thread.h
> --- runtime/mercury_thread.h	5 Jul 2006 03:00:43 -0000	1.19
> +++ runtime/mercury_thread.h	10 Jan 2007 01:59:18 -0000
> @@ -199,4 +199,46 @@
>
> extern	void    MR_finalize_thread_engine(void);
>
> -#endif
> +/*
> +** 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.
> +**
> +** 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.
> +*/
> +#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.

...

> 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
> @@ -584,13 +584,17 @@
>     MR_ticket_high_water = 1;
>   #endif
> #else
> -    /* start up the Mercury engine */
> -  #ifndef MR_THREAD_SAFE
> +    /*
> +    ** Start up the Mercury engine.  We don't yet know how many slots will be
> +    ** needed for thread-local mutable values so allocate the maximum number.
> +    */
>     MR_init_thread(MR_use_now);
> -  #else
> +    MR_SET_THREAD_LOCAL_MUTABLES(
> +        MR_NEW_ARRAY(MR_Word, MR_MAX_THREAD_LOCAL_MUTABLES));
> +
> +  #ifdef MR_THREAD_SAFE
>     {
>         int i;
> -        MR_init_thread(MR_use_now);
>         MR_exit_now = MR_FALSE;
>         for (i = 1 ; i < MR_num_threads ; i++) {
>             MR_create_thread(NULL);
> @@ -600,7 +604,7 @@
>         }
>     }
>   #endif /* ! MR_THREAD_SAFE */
> -#endif /* ! MR_HIGHLEVEL_CODE */
> +#endif /* ! 0 */

Why has this line changed?

I guess it's not really possible to add a test of these to the test
suite until spawn/3 is in the standard library.

The rest of that looks fine.

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