Bug 65556 – JSC GC uses dummy cells to avoid having to remember which cells it has already destroyed

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

The JSC GC attempts to maintain the invariant that every cell within a marked block has a C++ object that inherits from JSCell, so that anytime a sweep is performed, destructors can be called on all free cells even if this was done already.  This requires infrastructure for "dummy" JSCells, with "dummy" structures, just to allow the garbage collector to get around having to do bookkeeping about which parts of memory have already been swept.  Dummy cells should be replaced with a collector-internal mechanism for tracking which memory has already been swept.

[Filip Pizlo]

Created attachment 102674 [details]
the patch

[WebKit Review Bot]

Attachment 102674 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Source/JavaScriptCore/heap/MarkedBlock.cpp:217:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/JavaScriptCore/heap/MarkedBlock.h:173:  The parameter name "cell" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 102685 [details]
the patch

[Oliver Hunt]

Comment on attachment 102685 [details]
the patch

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

> Source/JavaScriptCore/heap/MarkedBlock.h:77
>              FreeCell* next;
> +            
> +            void setNoObject()
> +            {
> +                *reinterpret_cast<void**>(this) = 0;
> +            }

Why this cast void**?  FreeCell doesn't have a vtable pointer, you're simply assigning null to this->next?

[Filip Pizlo]

(In reply to comment #4)
> (From update of attachment 102685 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=102685&action=review
> 
> > Source/JavaScriptCore/heap/MarkedBlock.h:77
> >              FreeCell* next;
> > +            
> > +            void setNoObject()
> > +            {
> > +                *reinterpret_cast<void**>(this) = 0;
> > +            }
> 
> Why this cast void**?  FreeCell doesn't have a vtable pointer, you're simply assigning null to this->next?

I did that to emphasize the fact that I'm setting what-would-have-been-a-vtable to 0.  I suppose a comment would do the same job, so I'll post a fix.

[Filip Pizlo]

Created attachment 102690 [details]
the patch

[WebKit Review Bot]

Comment on attachment 102690 [details]
the patch

Clearing flags on attachment: 102690

Committed r92233: <http://trac.webkit.org/changeset/92233>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.