Bug 66265

Summary: Move allocation in constructors into separate constructorBody() methods
Product: WebKit Reporter: Mark Hahnenberg <mhahnenberg>
Component: JavaScriptCoreAssignee: Mark Hahnenberg <mhahnenberg>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren, oliver, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 65347    
Attachments:
Description Flags
Patch
none
Patch
none
Change function name
none
Fixing windows build errors none

Mark Hahnenberg
Reported 2011-08-15 16:51:08 PDT
This is the next small step in the process of refactoring JSC constructors to do no allocation in the middle of the execution of an initialization list. In this bug, we're going to pull the allocation done in the constructor bodies into separate finishAllocations() methods, which will in turn be called from the constructor bodies. This will set us up to have those finishAllocations() methods called outside of the constructor, allowing us to avoid allocation during initialization.
Attachments
Patch (44.48 KB, patch)
2011-08-15 16:55 PDT, Mark Hahnenberg 		no flags
DetailsFormatted DiffDiff
Patch (43.66 KB, patch)
2011-08-15 17:03 PDT, Mark Hahnenberg 		no flags
DetailsFormatted DiffDiff
Change function name (44.58 KB, patch)
2011-08-17 12:44 PDT, Mark Hahnenberg 		no flags
DetailsFormatted DiffDiff
Fixing windows build errors (45.57 KB, patch)
2011-08-18 16:02 PDT, Mark Hahnenberg 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Mark Hahnenberg
Comment 1 2011-08-15 16:55:40 PDT
Created attachment 103980 [details] Patch
Mark Hahnenberg
Comment 2 2011-08-15 17:03:10 PDT
Created attachment 103982 [details] Patch
Geoffrey Garen
Comment 3 2011-08-16 15:51:33 PDT
Comment on attachment 103982 [details] Patch Patch looks good. I think the simplest idiom to understand and enforce is "no constructor bodies or non-trivial initializers in GC objects." Then, this extra function you've added functions as the constructor body. So, let's call it "constructorBody". r=me with that change.
Mark Hahnenberg
Comment 4 2011-08-17 12:44:28 PDT
Created attachment 104216 [details] Change function name
Mark Hahnenberg
Comment 5 2011-08-18 16:02:44 PDT
Created attachment 104419 [details] Fixing windows build errors
Oliver Hunt
Comment 6 2011-08-18 16:08:15 PDT
Comment on attachment 104419 [details] Fixing windows build errors View in context: https://bugs.webkit.org/attachment.cgi?id=104419&action=review > Source/JavaScriptCore/runtime/JSPropertyNameIterator.h:90 > + PropertyNameArrayData::PropertyNameVector& propertyNameVector = propertyNameArrayData->propertyNameVector(); Out of curiosity, how does JSPropertyNameIterator handle gc during initialisation -- eg. if jsOwnedString triggers a gc?
Mark Hahnenberg
Comment 7 2011-08-18 16:15:33 PDT
> Out of curiosity, how does JSPropertyNameIterator handle gc during initialisation -- eg. if jsOwnedString triggers a gc? Poorly I think. visitChildren appends all of the values in m_jsStrings to the visitor, so if you're only halfway through creating the list, I think it has a potential to blow up.
WebKit Review Bot
Comment 8 2011-08-18 17:58:47 PDT
Comment on attachment 104419 [details] Fixing windows build errors Clearing flags on attachment: 104419 Committed r93378: <http://trac.webkit.org/changeset/93378>
WebKit Review Bot
Comment 9 2011-08-18 17:58:53 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.