Bug 129920 - MarkedBlocks that are "full enough" shouldn't be swept after EdenCollections
Summary: MarkedBlocks that are "full enough" shouldn't be swept after EdenCollections
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Hahnenberg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-07 14:34 PST by Mark Hahnenberg
Modified: 2014-03-12 09:22 PDT (History)
2 users (show)

See Also:


Attachments
Patch (12.35 KB, patch)
2014-03-11 16:50 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Patch (11.00 KB, patch)
2014-03-11 18:19 PDT, Mark Hahnenberg
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hahnenberg 2014-03-07 14:34:59 PST
Imagine a Heap this is completely full after an EdenCollection. The next allocation could potentially walk every atom of every MarkedBlock that its MarkedAllocator has. To avoid this situation we should ignore MarkedBlocks that are "full enough" so we can avoid this linear cost sweep phase.
Comment 1 Mark Hahnenberg 2014-03-11 16:50:41 PDT
Created attachment 226449 [details]
Patch
Comment 2 Mark Hahnenberg 2014-03-11 18:19:38 PDT
Created attachment 226458 [details]
Patch
Comment 3 Geoffrey Garen 2014-03-11 18:28:39 PDT
Comment on attachment 226458 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=226458&action=review

r=me

> Source/JavaScriptCore/heap/MarkedSpace.cpp:-317
> -#ifndef NDEBUG
> -    VerifyNewlyAllocated verifyFunctor;
> -    forEachBlock(verifyFunctor);
> -#endif

Why no verify?
Comment 4 Mark Hahnenberg 2014-03-11 18:33:01 PDT
(In reply to comment #3)
> (From update of attachment 226458 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=226458&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/heap/MarkedSpace.cpp:-317
> > -#ifndef NDEBUG
> > -    VerifyNewlyAllocated verifyFunctor;
> > -    forEachBlock(verifyFunctor);
> > -#endif
> 
> Why no verify?

I can undo that one. In an older version of the patch I was using m_newlyAllocated but no longer.
Comment 5 Mark Hahnenberg 2014-03-11 18:46:33 PDT
Committed r165458: <http://trac.webkit.org/changeset/165458>
Comment 6 Jer Noble 2014-03-12 09:22:19 PDT
This commit may have caused the fast/workers/worker-gc.html test to begin crashing <https://bugs.webkit.org/show_bug.cgi?id=130135>