Per-thread weak pointer list (remove global lock on mkWeak#)
Currently, we have to take out the storage manager lock when a weak pointer is allocated. If you're making a lot of weak pointers, this could become a bottleneck. We should maintain a per-thread weak pointer list per generation, so that we don't have to take out a lock.
Marking priority as low since I don't think anyone actually is trying to allocate tons of weak pointers on multiple threads, but I think this optimization should be relatively cheap. Probably a good starter ticket.
Trac metadata
| Trac field | Value |
|---|---|
| Version | 7.9 |
| Type | Bug |
| TypeOfFailure | OtherFailure |
| Priority | low |
| Resolution | Unresolved |
| Component | Runtime System |
| Test case | |
| Differential revisions | |
| BlockedBy | |
| Related | |
| Blocking | |
| CC | simonmar |
| Operating system | |
| Architecture |
Activity
added RTS Tbug Trac import newcomer labels
Suggested patch, still validating:
From 28cb3e18051db7d858bd913c002aedb78630bcdb Mon Sep 17 00:00:00 2001 From: "Edward Z. Yang" <ezyang@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@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.2mentioned in commit 723095b0
added runtime perf label
added Plow label
Please register or sign in to reply