Bug 90255

Summary: JSObject wastes too much memory on unused property slots
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Severity: Normal CC: zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 90336    
Bug Blocks:    
Description Flags
the patch
fpizlo: review-
the patch mhahnenberg: review+

Description Filip Pizlo 2012-06-29 00:43:12 PDT
1) Non-final objects should not have inline property storage, since it is used rarely and allocating out-of-line storage is cheap.

2) The initial size of out-of-line storage should not be a whopping 16 slots, or 128 bytes.  It should be some sensible amount, like maybe 4 (32 bytes), or 2x the inline storage, whichever is bigger.
Comment 1 Filip Pizlo 2012-06-29 00:49:18 PDT
Created attachment 150108 [details]
the patch
Comment 2 Mark Hahnenberg 2012-06-29 08:48:57 PDT
Comment on attachment 150108 [details]
the patch

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

Overall, looks good. r=me with suggestions :-)

> Source/JavaScriptCore/runtime/JSObject.h:351
> +#if JSNonFinalObject_inlineStorageCapacity

I guess this is to make it easier if we want to change back to non-zero inline storage for JSNonFinalObjects in the future?

> Source/JavaScriptCore/runtime/JSObject.h:361
> +                globalData, structure,

Why on a separate line?

> Source/JavaScriptCore/runtime/JSObject.h:574
> +    if (propertyStorageCapacity() == inlineStorageCapacity())

You could assert that propertyStorageCapacity() >= inlineStorageCapacity() at the beginning of the function and then return propertyStorageCapacity() == inlineStorageCapacity(). Two less lines!
Comment 3 Filip Pizlo 2012-06-29 17:07:00 PDT
Comment on attachment 150108 [details]
the patch

Found some bugs.  New patch on the way.
Comment 4 Filip Pizlo 2012-06-29 17:07:17 PDT
Created attachment 150276 [details]
the patch
Comment 5 Mark Hahnenberg 2012-06-29 17:23:15 PDT
Comment on attachment 150276 [details]
the patch

Comment 6 Filip Pizlo 2012-06-29 17:25:27 PDT
Landed in http://trac.webkit.org/changeset/121605
Comment 7 Zan Dobersek 2012-06-30 04:45:32 PDT
Re-opened since this is blocked by 90336
Comment 8 Filip Pizlo 2012-06-30 20:55:42 PDT
Relanded with fixes in http://trac.webkit.org/changeset/121633.