[m-rev.] for review: support STM in low level grades

[Paul Bone]

On Sun, May 03, 2009 at 10:26:53PM +1000, Ben Mellor wrote:
> Allow the Software Transactional Memory (STM) system to work in low level C
> grades.
> 
> runtime/mercury_stm.h
> 	In low level grades, define the type MR_STM_ConditionVar as an alias
> 	for MR_STM_TransLog*, and prototype the MR_STM_condvar_signal procedure.
> 
> runtime/mercury_stm.c
> 	MR_stm_block_thread is now conditional on MR_HIGHLEVEL_CODE.
> 
> 	In low level grades, MR_STM_condvar_signal is defined to allow the
> 	transaction logs saved as "condition variables" to be "signalled".
> 
> library/stm_builtin.m
> 	In low level grades, stm_block saves the current context in the waiter
> 	queues of the STM variables mentioned in the context's transaction log,
> 	then calls MR_runnext to look for more work.
> 
> 
> 
> Index: runtime/mercury_stm.c
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_stm.c,v
> retrieving revision 1.6
> diff -u -r1.6 mercury_stm.c
> --- runtime/mercury_stm.c	27 Feb 2008 07:23:57 -0000	1.6
> +++ runtime/mercury_stm.c	3 May 2009 12:23:54 -0000
> @@ -347,14 +347,22 @@
>  #endif
>  }
>  
> +
> +#if defined(MR_HIGHLEVEL_CODE)
> +/*
> +** MR_STM_block_thread is called to block the thread in high level C grades,
> +** using POSIX thread facilities, as there is a POSIX thread for every engine
> +** in these grades. The low level C grade equivalent of this code is defined
> +** in the stm_builtin library module.
> +*/
>  void
>  MR_STM_block_thread(MR_STM_TransLog *tlog)
>  {
>  #if defined(MR_THREAD_SAFE)
> -  #if defined(MR_HIGHLEVEL_CODE)
>          MR_STM_ConditionVar     *thread_condvar;
>  
>          thread_condvar = MR_GC_NEW(MR_STM_ConditionVar);
> +        MR_STM_condvar_init(thread_condvar);
>  
>          MR_STM_wait(tlog, thread_condvar);
>  
> @@ -370,11 +378,37 @@
>          MR_STM_unwait(tlog, thread_condvar);
>  
>          MR_GC_free(thread_condvar);
> -  #else
> -        MR_fatal_error("Low-Level backend: Not implemented");
> -  #endif
>  #else
>      MR_fatal_error("Blocking thread in non-parallel grade");
>  #endif
> +}
> +#endif
> +
> +
> +#if !defined(MR_HIGHLEVEL_CODE)
> +/*
> +** In the low level C grades, the "condition variable" created when an STM
> +** transaction blocks is actually a pointer to the transaction log.
> "Signalling" +** it consists of going through the STM variables listed in the
> log and removing +** the waiters attached to them for the context listed in the
> log. After this, +** the context can be safely rescheduled.
> +*/

Are these over-long lines?

> +void
> +MR_STM_condvar_signal(MR_STM_ConditionVar *cvar)
> +{
> +    /*
> +    ** Calling MR_STM_unwait here should be safe, as this signalling is called
> +    ** in response to a commit, while the committing thread holds the global
> +    ** STM lock. Note that a MR_STM_ConditionVar IS a MR_STM_TransLog if
> +    ** MR_HIGHLEVEL_CODE is not defined, which is why cvar is passed twice.
> +    */
> +    MR_STM_unwait(cvar, cvar);
> +
> +#if defined(MR_STM_DEBUG)
> +        fprintf(stderr, "STM RESCHEDULING: log <0x%.8lx>\n", (MR_Word)cvar);
> +#endif
>  
> +    MR_schedule_context(MR_STM_context_from_condvar(cvar));
>  }
> +
> +#endif

Please add a warning to the prototype for MR_STM_condvar_signal to inform
anyone calling this function that they must first have the global STM lock as
mentioned in your comment.

> Index: runtime/mercury_stm.h
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_stm.h,v
> retrieving revision 1.6
> diff -u -r1.6 mercury_stm.h
> --- runtime/mercury_stm.h	27 Feb 2008 07:23:57 -0000	1.6
> +++ runtime/mercury_stm.h	3 May 2009 12:23:54 -0000
> @@ -78,6 +78,7 @@
>      #if defined(MR_THREAD_SAFE)
>          typedef MercuryCond  MR_STM_ConditionVar;
>  
> +        #define MR_STM_condvar_init(x)        pthread_cond_init(x,
> MR_COND_ATTR) #define MR_STM_condvar_wait(x, y)     MR_cond_wait(x, y)
>          #define MR_STM_condvar_signal(x)      MR_cond_signal(x)
>      #else
> @@ -86,20 +87,18 @@
>          ** Since these grades don't support concurrency, there is no
>          ** need to block the thread.
>          */
> +        #define MR_STM_condvar_init(x)
>          #define MR_STM_condvar_wait(x, y)
>          #define MR_STM_condvar_signal(x)
>      #endif
>  
>  #else /* !MR_HIGHLEVEL_CODE */
>  
> -    typedef MR_Context  *MR_STM_ConditionVar;
> +    typedef MR_STM_TransLog  MR_STM_ConditionVar;
>  
> -    /*
> -    ** These are dummy definitions; STM is not yet implemented for low level C
> -    ** grades.
> -    */
> -    #define MR_STM_condvar_wait(x, y)
> -    #define MR_STM_condvar_signal(x)
> +    extern void MR_STM_condvar_signal(MR_STM_ConditionVar *cvar);
> +
> +    #define MR_STM_context_from_condvar(x)      (x->MR_STM_tl_thread)
>  
>  #endif /* !MR_HIGHLEVEL_CODE */

Why is MR_STM_condvar_signal defined as 'extern'?

> Index: library/stm_builtin.m
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/library/stm_builtin.m,v
> retrieving revision 1.14
> diff -u -r1.14 stm_builtin.m
> --- library/stm_builtin.m	26 Feb 2009 23:43:08 -0000	1.14
> +++ library/stm_builtin.m	3 May 2009 12:23:54 -0000
> @@ -368,13 +368,100 @@
>  
>  %-----------------------------------------------------------------------------%
>  
> +
> +% In high level C grades, stm_block calls a C procedure in the runtime that
> +% blocks the POSIX thread until signalled, as there is one POSIX thread for
> +% every Mercury thread.
> +% In the low level C grades, this approach cannot be taken, as when the context
> +% blocks the engine running in the POSIX thread needs to be able to look for
> +% other work (there might only be a single engine/POSIX thread).
> +
>  :- pragma foreign_proc("C",
>      stm_block(STM::ui),
>      [will_not_call_mercury, thread_safe],
>  "
> +#if defined(MR_HIGHLEVEL_CODE)
>      MR_STM_block_thread(STM);
> +#else
> +
> +    MR_STM_wait(STM, STM);
> +
> +#if defined(MR_STM_DEBUG)
> +        fprintf(stderr, ""STM BLOCKING: log <0x%.8lx>\n"", STM);
> +#endif
> +
> +    MR_save_context(MR_ENGINE(MR_eng_this_context));
> +
> +    MR_ENGINE(MR_eng_this_context)->MR_ctxt_resume =
> +        MR_ENTRY(mercury__stm_builtin__block_thread_resume);
> +
> +    MR_ENGINE(MR_eng_this_context) = NULL;
> +    MR_UNLOCK(&MR_STM_lock, ""MR_STM_block_thread"");
> +    MR_runnext();
> +
> +#endif
>  ").
>  
> +
> +% block_thread_resume is the piece of code we jump to when a thread suspended
> +% after a failed STM transaction resumes
> +:- pragma foreign_decl("C",
> +"
> +/*
> +INIT mercury_sys_init_stm_builtin_modules
> +*/
> +
> +#if (!defined MR_HIGHLEVEL_CODE)
> +    MR_define_extern_entry(mercury__stm_builtin__block_thread_resume);
> +#endif
> +").
> +
> +:- pragma foreign_code("C",
> +"
> +#if (!defined MR_HIGHLEVEL_CODE)
> +
> +    MR_BEGIN_MODULE(hand_written_stm_builtin_module)
> +        MR_init_entry_ai(mercury__stm_builtin__block_thread_resume);
> +    MR_BEGIN_CODE
> +
> +    MR_define_entry(mercury__stm_builtin__block_thread_resume);
> +    {
> +        MR_proceed();
> +    }
> +    MR_END_MODULE
> +
> +#endif
> +
> +    /* forward decls to suppress gcc warnings */
> +    void mercury_sys_init_stm_builtin_modules_init(void);
> +    void mercury_sys_init_stm_builtin_modules_init_type_tables(void);
> +    #ifdef  MR_DEEP_PROFILING
> +    void mercury_sys_init_stm_builtin_modules_write_out_proc_statics(
> +        FILE *deep_fp, FILE *procrep_fp);
> +    #endif
> +
> +    void mercury_sys_init_stm_builtin_modules_init(void)
> +    {
> +    #if (!defined MR_HIGHLEVEL_CODE)
> +        hand_written_stm_builtin_module();
> +    #endif
> +    }
> +
> +    void mercury_sys_init_stm_builtin_modules_init_type_tables(void)
> +    {
> +        /* no types to register */
> +    }
> +
> +    #ifdef  MR_DEEP_PROFILING
> +    void mercury_sys_init_stm_builtin_modules_write_out_proc_statics(
> +        FILE *deep_fp, FILE *procrep_fp)
> +    {
> +        /* no proc_statics to write out */
> +    }
> +    #endif
> +").
> +
> +

The rest seems fine, but I don't know enough about the STM implementation to be
sure.  So I'm not 100% confident.  You can probably commit it though.

Cheers.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20090504/254193c4/attachment.sig>