Bug 21683 - Don't create intermediate StructureIDs for builtin objects
Summary: Don't create intermediate StructureIDs for builtin objects
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 21513
  Show dependency treegraph
 
Reported: 2008-10-16 15:41 PDT by Sam Weinig
Modified: 2010-11-04 03:30 PDT (History)
1 user (show)

See Also:


Attachments
Patch 1 - Just ObjectPrototype (13.03 KB, patch)
2008-10-16 16:03 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch 2 - A different approach (12.05 KB, patch)
2008-10-16 18:59 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch 3 - The remaining singletons (41.28 KB, patch)
2008-10-17 17:09 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2008-10-16 15:41:28 PDT
We currently create a lot of intermediate StructureIDs when creating the JSGlobalObject due to a plethora of calls to putDirect, which transition the StructureIDs even though no other objects can ever share it.  Getting rid of these should reduce memory and speed things up as well.
Comment 1 Sam Weinig 2008-10-16 15:42:32 PDT
As a starting point, we currently allocate 142 StrucutureIDs for about:blank.
Comment 2 Sam Weinig 2008-10-16 16:03:18 PDT
Created attachment 24436 [details]
Patch 1 - Just ObjectPrototype

This patch implements the technique to just reduce the number of StructureIDs for the ObjectPrototype class.  It brings down the number of live StructureIDs on about:blank from 142 to 132.
Comment 3 Geoffrey Garen 2008-10-16 16:22:12 PDT
Comment on attachment 24436 [details]
Patch 1 - Just ObjectPrototype

Clearing review flag because Sam is working on another approach.
Comment 4 Geoffrey Garen 2008-10-16 16:22:13 PDT
Comment on attachment 24436 [details]
Patch 1 - Just ObjectPrototype

Clearing review flag because Sam is working on another approach.
Comment 5 Sam Weinig 2008-10-16 18:59:49 PDT
Created attachment 24450 [details]
Patch 2 - A different approach
Comment 6 Sam Weinig 2008-10-16 19:00:34 PDT
I obviously would not land this with #define DUMP_STRUCTURE_ID_STATISTICS 1
Comment 7 Maciej Stachowiak 2008-10-16 19:12:40 PDT
Probably don't want to check this in:

-#define DUMP_STRUCTURE_ID_STATISTICS 0
+#define DUMP_STRUCTURE_ID_STATISTICS 1

Comment 8 Maciej Stachowiak 2008-10-16 19:20:45 PDT
Comment on attachment 24450 [details]
Patch 2 - A different approach

I like this approach. Please perf test and disable the logging.

r=me, with changes noted above.
Comment 9 Sam Weinig 2008-10-16 19:59:26 PDT
Comment on attachment 24450 [details]
Patch 2 - A different approach

Clearing the review flag.  This was landed in r37645.
Comment 10 Sam Weinig 2008-10-17 17:09:08 PDT
Created attachment 24469 [details]
Patch 3 - The remaining singletons
Comment 11 Cameron Zwarich (cpst) 2008-10-17 18:09:45 PDT
Comment on attachment 24469 [details]
Patch 3 - The remaining singletons

r=me
Comment 12 Sam Weinig 2008-10-20 14:28:22 PDT
Comment on attachment 24469 [details]
Patch 3 - The remaining singletons

Clearing the review flag.  This was landed in r37747.
Comment 13 Xan Lopez 2010-11-01 15:06:45 PDT
(In reply to comment #10)
> Created an attachment (id=24469) [details]
> Patch 3 - The remaining singletons

It seems the stuff in this patch was eventually landed, so should we close this bug?
Comment 14 Sam Weinig 2010-11-04 03:30:40 PDT
Heh, yeah.