Bug 120898

Summary: Stop using fastNew/fastDelete in JavaScriptCore
Product: WebKit Reporter: Anders Carlsson <andersca>
Component: New BugsAssignee: Anders Carlsson <andersca>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, ggaren, kling
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch oliver: review+

Description Anders Carlsson 2013-09-06 15:05:45 PDT
Stop using fastNew/fastDelete in JavaScriptCore
Comment 1 Anders Carlsson 2013-09-06 15:10:28 PDT
Created attachment 210804 [details]
Patch
Comment 2 Andreas Kling 2013-09-06 15:15:55 PDT
Comment on attachment 210804 [details]
Patch

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

> Source/JavaScriptCore/runtime/VM.cpp:160
> +    , arrayConstructorTable(adoptPtr(new HashTable(JSC::arrayConstructorTable)))
> +    , arrayPrototypeTable(adoptPtr(new HashTable(JSC::arrayPrototypeTable)))
> +    , booleanPrototypeTable(adoptPtr(new HashTable(JSC::booleanPrototypeTable)))

Perhaps we should add a HashTable::create() helper that returns a PassOwnPtr?
We don't want to put these on the stack anywhere right?

> Source/JavaScriptCore/runtime/VM.h:227
> +        OwnPtr<const HashTable> arrayConstructorTable;
> +        OwnPtr<const HashTable> arrayPrototypeTable;
> +        OwnPtr<const HashTable> booleanPrototypeTable;

You could make these "const OwnPtr<const HashTable>" to prevent changes post-construction.
Comment 3 Anders Carlsson 2013-09-06 15:18:15 PDT
(In reply to comment #2)
> (From update of attachment 210804 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=210804&action=review
> 
> > Source/JavaScriptCore/runtime/VM.cpp:160
> > +    , arrayConstructorTable(adoptPtr(new HashTable(JSC::arrayConstructorTable)))
> > +    , arrayPrototypeTable(adoptPtr(new HashTable(JSC::arrayPrototypeTable)))
> > +    , booleanPrototypeTable(adoptPtr(new HashTable(JSC::booleanPrototypeTable)))
> 
> Perhaps we should add a HashTable::create() helper that returns a PassOwnPtr?
> We don't want to put these on the stack anywhere right?

I’d rather we add a makeOwned function template so you can do

arrayConstructorTable(makeOwned<HashTable>())

and have things just work. I want to do that after we’ve gotten rid of PassOwnPtr though.

> 
> > Source/JavaScriptCore/runtime/VM.h:227
> > +        OwnPtr<const HashTable> arrayConstructorTable;
> > +        OwnPtr<const HashTable> arrayPrototypeTable;
> > +        OwnPtr<const HashTable> booleanPrototypeTable;
> 
> You could make these "const OwnPtr<const HashTable>" to prevent changes post-construction.

Good idea!
Comment 4 Anders Carlsson 2013-09-06 15:31:15 PDT
Committed r155219: <http://trac.webkit.org/changeset/155219>