Opened 16 months ago

Closed 8 months ago

Last modified 8 months ago

#9067 closed task (fixed)

Optimize clearNursery by short-circuiting when we get to currentNursery

Reported by: ezyang Owned by: ezyang
Priority: low Milestone: 7.10.1
Component: Runtime System Version: 7.9
Keywords: Cc: simonmar
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Runtime performance bug Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions: Phab:D318

Description

This is a note to myself so I don't forget about this. Essentially, we can do something like this (this particular patch variant untested):

diff --git a/rts/sm/Storage.c b/rts/sm/Storage.c
index 36776b9..0311042 100644
--- a/rts/sm/Storage.c
+++ b/rts/sm/Storage.c
@@ -598,6 +598,11 @@ clearNursery (Capability *cap)
         ASSERT(bd->gen_no == 0);
         ASSERT(bd->gen == g0);
         IF_DEBUG(sanity,memset(bd->start, 0xaa, BLOCK_SIZE));
+        if (bd == cap->r.rCurrentNursery) {
+            IF_DEBUG(sanity, for (bd = bd->link; bd; bd = bd->link)
+                                ASSERT(bd->free == bd->start));
+            break;
+        }
     }
     }
 }

This is due to invariants about how we manage the currentNursery pointer. But we need a note about it, and I need to test it more carefully. This optimization probably doesn't help too much on normal GHC, but when I have lots of nurseries it helps quite a bit.

Change History (4)

comment:1 Changed 16 months ago by simonmar

Andreas Voellmy and I talked about some similar ideas. I doubt that you want to rely on CurrentNursery, because we want to reset that after every GC, but instead I think the right approach is to assume that every block from CurrentNursery onwards needs to have its free pointer reset before we use it. It's a somewhat subtle invariant and something else we have to get right, but the win seems to be worthwhile.

comment:2 Changed 11 months ago by simonmar

  • Differential Revisions set to Phab:D318

comment:3 Changed 8 months ago by ezyang

  • Resolution set to fixed
  • Status changed from new to closed
commit e22bc0dedb9e9da0176ad7ce4a74acbefedc7207
Author: Simon Marlow <[email protected]>
Date:   Tue Oct 7 10:30:36 2014 +0100

    Make clearNursery free
    
    Summary:
    clearNursery resets all the bd->free pointers of nursery blocks to
    make the blocks empty.  In profiles we've seen clearNursery taking
    significant amounts of time particularly with large -N and -A values.
    
    This patch moves the work of clearNursery to the point at which we
    actually need the new block, thereby introducing an invariant that
    blocks to the right of the CurrentNursery pointer still need their
    bd->free pointer reset.  This should make things faster overall,
    because we don't need to clear blocks that we don't use.
    
    Test Plan: validate
    
    Reviewers: AndreasVoellmy, ezyang, austin
    
    Subscribers: thomie, carter, ezyang, simonmar
    
    Differential Revision: https://phabricator.haskell.org/D318

comment:4 Changed 8 months ago by ezyang

  • Milestone set to 7.10.1
Note: See TracTickets for help on using tickets.