Bug 129464

Summary:

Clean up Heap::collect and Heap::markRoots

Product:

WebKit

Reporter:

Mark Hahnenberg <mhahnenberg>

Component:

JavaScriptCore

Assignee:

Mark Hahnenberg <mhahnenberg>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ossy

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

Mark Hahnenberg

Reported 2014-02-27 16:46:30 PST

These functions have built up a lot of cruft recently. We should do a bit of cleanup to make them easier to grok.

Attachments
Patch (28.63 KB, patch) 2014-02-27 16:49 PST, Mark Hahnenberg	no flags	Details Formatted Diff Diff
Patch (1.35 KB, patch) 2014-02-28 09:35 PST, Mark Hahnenberg	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Mark Hahnenberg

Comment 1 2014-02-27 16:49:57 PST

Created attachment 225430 [details] Patch

Geoffrey Garen

Comment 2 2014-02-27 18:46:49 PST

Comment on attachment 225430 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=225430&action=review r=me > Source/JavaScriptCore/heap/Heap.cpp:483 > +void Heap::markProtectedObjects(HeapRootVisitor& heapRootVisitor) "visit"? > Source/JavaScriptCore/heap/Heap.cpp:493 > +void Heap::markTempSortVectors(HeapRootVisitor& heapRootVisitor) "visit"? > Source/JavaScriptCore/heap/Heap.cpp:513 > +void Heap::markArgumentBuffers(HeapRootVisitor& visitor) "visit"? > Source/JavaScriptCore/heap/Heap.cpp:637 > + SamplingRegion samplingRegion("Garbage Collection: Tracing"); "Marking"?

Mark Hahnenberg

Comment 3 2014-02-28 08:50:48 PST

Committed r164862: <http://trac.webkit.org/changeset/164862>

Mark Hahnenberg

Comment 4 2014-02-28 08:51:05 PST

Comment on attachment 225430 [details] Patch Clearing review flag.

Csaba Osztrogonác

Comment 5 2014-02-28 09:05:46 PST

Comment on attachment 225430 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=225430&action=review > Source/JavaScriptCore/heap/Heap.cpp:667 > +#if ENABLE(GGC) > + Vector<const JSCell*> rememberedSet(m_slotVisitor.markStack().size()); > + m_slotVisitor.markStack().fillVector(rememberedSet); > +#endif rememberedSet is defined inside ENABLE(GGC) guard ... > Source/JavaScriptCore/heap/Heap.cpp:689 > + clearRememberedSet(rememberedSet); ... but used outside. And it broke the !ENABLE(GGC) builds.

Mark Hahnenberg

Comment 6 2014-02-28 09:21:56 PST

(In reply to comment #5) > (From update of attachment 225430 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=225430&action=review > > > Source/JavaScriptCore/heap/Heap.cpp:667 > > +#if ENABLE(GGC) > > + Vector<const JSCell*> rememberedSet(m_slotVisitor.markStack().size()); > > + m_slotVisitor.markStack().fillVector(rememberedSet); > > +#endif > > rememberedSet is defined inside ENABLE(GGC) guard ... > > > Source/JavaScriptCore/heap/Heap.cpp:689 > > + clearRememberedSet(rememberedSet); > > ... but used outside. And it broke the !ENABLE(GGC) builds. Good point, fix coming.

Mark Hahnenberg

Comment 7 2014-02-28 09:35:40 PST

Reopening to attach new patch.

Mark Hahnenberg

Comment 8 2014-02-28 09:35:49 PST

Created attachment 225473 [details] Patch

Mark Hahnenberg

Comment 9 2014-02-28 09:38:53 PST

Comment on attachment 225473 [details] Patch Since this fixes a build, it doesn't need review but feel free to comment :-)

Mark Hahnenberg

Comment 10 2014-02-28 09:41:10 PST

Comment on attachment 225473 [details] Patch Bah, commit queue will take too long. Landed manually in r164864.

Mark Hahnenberg

Comment 11 2014-03-04 21:51:27 PST

Closing.

Note You need to log in before you can comment on or make changes to this bug.