[GHC] #9075: Per-thread weak pointer list (remove global lock on mkWeak#)

GHC ghc-devs at haskell.org
Thu May 29 07:21:50 UTC 2014


#9075: Per-thread weak pointer list (remove global lock on mkWeak#)
--------------------------------------------+------------------------------
        Reporter:  ezyang                   |            Owner:  ezyang
            Type:  bug                      |           Status:  new
        Priority:  low                      |        Milestone:
       Component:  Runtime System           |          Version:  7.9
      Resolution:                           |         Keywords:  easy
Operating System:  Unknown/Multiple         |     Architecture:
 Type of failure:  Runtime performance bug  |  Unknown/Multiple
       Test Case:                           |       Difficulty:  Unknown
        Blocking:                           |       Blocked By:
                                            |  Related Tickets:
--------------------------------------------+------------------------------

Comment (by ezyang):

 Suggested patch, still validating:

 {{{
 From 28cb3e18051db7d858bd913c002aedb78630bcdb Mon Sep 17 00:00:00 2001
 From: "Edward Z. Yang" <ezyang at cs.stanford.edu>
 Date: Thu, 29 May 2014 00:18:33 -0700
 Subject: [PATCH] Per-capability nursery weak pointer lists.

 Signed-off-by: Edward Z. Yang <ezyang at cs.stanford.edu>
 ---
  rts/Capability.c                         |  2 ++
  rts/Capability.h                         |  5 +++++
  rts/PrimOps.cmm                          |  9 +++++----
  rts/RetainerProfile.c                    |  6 ++++++
  rts/RtsStartup.c                         |  5 ++++-
  rts/sm/GC.c                              |  1 +
  rts/sm/MarkWeak.c                        | 18 ++++++++++++++++++
  rts/sm/MarkWeak.h                        |  1 +
  utils/deriveConstants/DeriveConstants.hs |  2 ++
  9 files changed, 44 insertions(+), 5 deletions(-)

 diff --git a/rts/Capability.c b/rts/Capability.c
 index 16b71b7..805a35b 100644
 --- a/rts/Capability.c
 +++ b/rts/Capability.c
 @@ -273,6 +273,8 @@ initCapability( Capability *cap, nat i )
         cap->mut_lists[g] = NULL;
      }

 +    cap->weak_ptr_list_hd = NULL;
 +    cap->weak_ptr_list_tl = NULL;
      cap->free_tvar_watch_queues = END_STM_WATCH_QUEUE;
      cap->free_invariant_check_queues = END_INVARIANT_CHECK_QUEUE;
      cap->free_trec_chunks = END_STM_CHUNK_LIST;
 diff --git a/rts/Capability.h b/rts/Capability.h
 index f342d92..d36d502 100644
 --- a/rts/Capability.h
 +++ b/rts/Capability.h
 @@ -79,6 +79,11 @@ struct Capability_ {
      // full pinned object blocks allocated since the last GC
      bdescr *pinned_object_blocks;

 +    // per-capability weak pointer list associated with nursery (older
 +    // lists stored in generation object)
 +    StgWeak *weak_ptr_list_hd;
 +    StgWeak *weak_ptr_list_tl;
 +
      // Context switch flag.  When non-zero, this means: stop running
      // Haskell code, and switch threads.
      int context_switch;
 diff --git a/rts/PrimOps.cmm b/rts/PrimOps.cmm
 index 1dc232d..84bcea5 100644
 --- a/rts/PrimOps.cmm
 +++ b/rts/PrimOps.cmm
 @@ -577,10 +577,11 @@ stg_mkWeakzh ( gcptr key,
      StgWeak_finalizer(w)   = finalizer;
      StgWeak_cfinalizers(w) = stg_NO_FINALIZER_closure;

 -    ACQUIRE_LOCK(sm_mutex);
 -    StgWeak_link(w) = generation_weak_ptr_list(W_[g0]);
 -    generation_weak_ptr_list(W_[g0]) = w;
 -    RELEASE_LOCK(sm_mutex);
 +    StgWeak_link(w) = Capability_weak_ptr_list_hd(MyCapability());
 +    Capability_weak_ptr_list_hd(MyCapability()) = w;
 +    if (Capability_weak_ptr_list_tl(MyCapability()) == NULL) {
 +        Capability_weak_ptr_list_tl(MyCapability()) = w;
 +    }

      IF_DEBUG(weak, ccall debugBelch(stg_weak_msg,w));

 diff --git a/rts/RetainerProfile.c b/rts/RetainerProfile.c
 index bdfc831..bfc9624 100644
 --- a/rts/RetainerProfile.c
 +++ b/rts/RetainerProfile.c
 @@ -1781,6 +1781,12 @@ computeRetainerSet( void )
      //
      // The following code assumes that WEAK objects are considered to be
 roots
      // for retainer profilng.
 +    for (n = 0; n < n_capabilities; n++) {
 +        // NB: after a GC, all nursery weak_ptr_lists have been migrated
 +        // to the global lists living in the generations
 +        ASSERT(capabilities[n]->weak_ptr_list_hd == NULL);
 +        ASSERT(capabilities[n]->weak_ptr_list_tl == NULL);
 +    }
      for (g = 0; g < RtsFlags.GcFlags.generations; g++) {
          for (weak = generations[g].weak_ptr_list; weak != NULL; weak =
 weak->link) {
              // retainRoot((StgClosure *)weak);
 diff --git a/rts/RtsStartup.c b/rts/RtsStartup.c
 index 15e48a6..06e888c 100644
 --- a/rts/RtsStartup.c
 +++ b/rts/RtsStartup.c
 @@ -304,7 +304,7 @@ hs_add_root(void (*init_root)(void) STG_UNUSED)
  static void
  hs_exit_(rtsBool wait_foreign)
  {
 -    nat g;
 +    nat g, i;

      if (hs_init_count <= 0) {
         errorBelch("warning: too many hs_exit()s");
 @@ -336,6 +336,9 @@ hs_exit_(rtsBool wait_foreign)
      exitScheduler(wait_foreign);

      /* run C finalizers for all active weak pointers */
 +    for (i = 0; i < n_capabilities; i++) {
 +        runAllCFinalizers(capabilities[i]->weak_ptr_list_hd);
 +    }
      for (g = 0; g < RtsFlags.GcFlags.generations; g++) {
          runAllCFinalizers(generations[g].weak_ptr_list);
      }
 diff --git a/rts/sm/GC.c b/rts/sm/GC.c
 index d22a31e..2122385 100644
 --- a/rts/sm/GC.c
 +++ b/rts/sm/GC.c
 @@ -378,6 +378,7 @@ GarbageCollect (nat collect_gen,
  #endif

    // Mark the weak pointer list, and prepare to detect dead weak
 pointers.
 +  collectFreshWeakPtrs();
    markWeakPtrList();
    initWeakForGC();

 diff --git a/rts/sm/MarkWeak.c b/rts/sm/MarkWeak.c
 index af953cd..1a7359f 100644
 --- a/rts/sm/MarkWeak.c
 +++ b/rts/sm/MarkWeak.c
 @@ -341,6 +341,24 @@ static void tidyThreadList (generation *gen)
      }
  }

 +void collectFreshWeakPtrs()
 +{
 +    nat i;
 +    generation *gen = &generations[0];
 +    // move recently allocated weak_ptr_list to the old list as well
 +    for (i = 0; i < n_capabilities; i++) {
 +        Capability *cap = capabilities[i];
 +        if (cap->weak_ptr_list_tl != NULL) {
 +            cap->weak_ptr_list_tl->link = gen->weak_ptr_list;
 +            gen->weak_ptr_list = cap->weak_ptr_list_hd;
 +            cap->weak_ptr_list_tl = NULL;
 +            cap->weak_ptr_list_hd = NULL;
 +        } else {
 +            ASSERT(cap->weak_ptr_list_hd == NULL);
 +        }
 +    }
 +}
 +
  /*
 -----------------------------------------------------------------------------
     Evacuate every weak pointer object on the weak_ptr_list, and update
     the link fields.
 diff --git a/rts/sm/MarkWeak.h b/rts/sm/MarkWeak.h
 index f9bacfa..bd0231d 100644
 --- a/rts/sm/MarkWeak.h
 +++ b/rts/sm/MarkWeak.h
 @@ -20,6 +20,7 @@ extern StgWeak *old_weak_ptr_list;
  extern StgTSO *resurrected_threads;
  extern StgTSO *exception_threads;

 +void    collectFreshWeakPtrs   ( void );
  void    initWeakForGC          ( void );
  rtsBool traverseWeakPtrList    ( void );
  void    markWeakPtrList        ( void );
 diff --git a/utils/deriveConstants/DeriveConstants.hs
 b/utils/deriveConstants/DeriveConstants.hs
 index d15f619..9bf2160 100644
 --- a/utils/deriveConstants/DeriveConstants.hs
 +++ b/utils/deriveConstants/DeriveConstants.hs
 @@ -349,6 +349,8 @@ wanteds = concat
            ,structField C    "Capability" "context_switch"
            ,structField C    "Capability" "interrupt"
            ,structField C    "Capability" "sparks"
 +          ,structField C    "Capability" "weak_ptr_list_hd"
 +          ,structField C    "Capability" "weak_ptr_list_tl"

            ,structField Both "bdescr" "start"
            ,structField Both "bdescr" "free"
 --
 1.9.2


 }}}

--
Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/9075#comment:3>
GHC <http://www.haskell.org/ghc/>
The Glasgow Haskell Compiler


More information about the ghc-tickets mailing list