Bug 21683

Summary: Don't create intermediate StructureIDs for builtin objects
Product: WebKit Reporter: Sam Weinig <sam>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 21513    
Attachments:
Description Flags
Patch 1 - Just ObjectPrototype
none
Patch 2 - A different approach
none
Patch 3 - The remaining singletons none

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.