Bug 90255 - JSObject wastes too much memory on unused property slots
Summary: JSObject wastes too much memory on unused property slots
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
Depends on: 90336
  Show dependency treegraph
Reported: 2012-06-29 00:43 PDT by Filip Pizlo
Modified: 2012-06-30 20:55 PDT (History)
1 user (show)

See Also:

the patch (20.00 KB, patch)
2012-06-29 00:49 PDT, Filip Pizlo
fpizlo: review-
Details | Formatted Diff | Diff
the patch (26.84 KB, patch)
2012-06-29 17:07 PDT, Filip Pizlo
mhahnenberg: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.