Bug 92679

Summary: Structures should be swept after all other objects
Product: WebKit Reporter: Mark Hahnenberg <mhahnenberg>
Component: JavaScriptCoreAssignee: Mark Hahnenberg <mhahnenberg>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, ggaren, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 92700    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch fpizlo: review+

Description Mark Hahnenberg 2012-07-30 14:25:19 PDT
In order to get rid of ClassInfo from our objects, we need to be able to safely get the ClassInfo during the destruction of objects. We'd like to get the ClassInfo out of the Structure, but currently it is not safe to do so because the order of destruction of objects is not guaranteed to sweep objects before their corresponding Structure. We can fix this by sweeping Structures after everything else.
Comment 1 Mark Hahnenberg 2012-07-30 15:57:05 PDT
Created attachment 155389 [details]
Patch
Comment 2 Filip Pizlo 2012-07-30 17:26:50 PDT
Comment on attachment 155389 [details]
Patch

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

> Source/JavaScriptCore/heap/HeapTimer.h:44
> +

Kill it with fire!

> Source/JavaScriptCore/heap/IncrementalSweeper.cpp:74
> +    return !m_blocksToSweep.size() || m_currentBlockToSweepIndex >= m_blocksToSweep.size();

I can has assert that m_currentBlockToSweepIndex <= m_blocksToSweep.size()
Comment 3 Mark Hahnenberg 2012-07-30 17:34:09 PDT
Committed r124123: <http://trac.webkit.org/changeset/124123>
Comment 4 Geoffrey Garen 2012-07-30 18:03:01 PDT
Comment on attachment 155389 [details]
Patch

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

> Source/JavaScriptCore/heap/IncrementalSweeper.cpp:83
> +        MarkedBlock* block = m_blocksToSweep[m_currentBlockToSweepIndex];
> +        if (block->onlyContainsStructures()) {
> +            m_currentBlockToSweepIndex++;
> +            continue;

This logic gets a little whacky. Can't we just arrange for all the Structure blocks to be added to the array last, so we naturally sweep them last?
Comment 5 WebKit Review Bot 2012-07-30 18:37:53 PDT
Re-opened since this is blocked by 92700
Comment 6 Mark Hahnenberg 2012-07-31 13:25:20 PDT
Created attachment 155621 [details]
Patch
Comment 7 Mark Hahnenberg 2012-07-31 16:05:29 PDT
Committed r124265: <http://trac.webkit.org/changeset/124265>