Bug 107378

Summary: Track inheritance structures in a side table, instead of using a private name in each prototype
Product: WebKit Reporter: Geoffrey Garen <ggaren>
Component: New BugsAssignee: Geoffrey Garen <ggaren>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, gyuyoung.kim, rakuco, sam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ggaren: review+
performance none

Geoffrey Garen
Reported 2013-01-19 13:08:23 PST
Track inheritance structures in a side table, instead of using a private name in each prototype
Attachments
Patch (29.87 KB, patch)
2013-01-19 13:26 PST, Geoffrey Garen 		ggaren: review+
DetailsFormatted DiffDiff
performance (19.07 KB, text/plain)
2013-01-19 13:28 PST, Geoffrey Garen 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Geoffrey Garen
Comment 1 2013-01-19 13:26:15 PST
Created attachment 183632 [details] Patch
Geoffrey Garen
Comment 2 2013-01-19 13:28:10 PST
Created attachment 183633 [details] performance
Geoffrey Garen
Comment 3 2013-01-19 13:28:50 PST
Seems performance-neutral, as expected.
Geoffrey Garen
Comment 4 2013-01-19 13:48:47 PST
Comment on attachment 183632 [details] Patch Sam and Phil reviewed this.
Geoffrey Garen
Comment 5 2013-01-19 13:49:02 PST
Committed r140259: <http://trac.webkit.org/changeset/140259>
Geoffrey Garen
Comment 7 2013-01-20 12:35:10 PST
Disabled the ASSERT for the time being to get the bots green: <http://trac.webkit.org/changeset/140259>.
Geoffrey Garen
Comment 8 2013-01-20 17:01:24 PST
Follow-up ASSERT fix: <http://trac.webkit.org/changeset/140284>.
Darin Adler
Comment 9 2013-01-21 10:07:14 PST
Comment on attachment 183632 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183632&action=review > Source/JavaScriptCore/runtime/PrototypeMap.h:41 > + bool isPrototype(JSObject*); // Returns a conservative estimate. Two thoughts: 1) If it’s a conservative estimate, then how about a name that reflects that? Perhaps mayBePrototype? I’m not really sure what a conservative estimate means. 2) Should be a const member function.
Filip Pizlo
Comment 10 2013-01-21 10:58:22 PST
(In reply to comment #9) > (From update of attachment 183632 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=183632&action=review > > > Source/JavaScriptCore/runtime/PrototypeMap.h:41 > > + bool isPrototype(JSObject*); // Returns a conservative estimate. > > Two thoughts: > > 1) If it’s a conservative estimate, then how about a name that reflects that? Perhaps mayBePrototype? I’m not really sure what a conservative estimate means. I like mayBePrototype. But I think the reason why this was landed as isPrototype is that we were thinking of converting all of these possibly-conservative Boolean queries to use TriState. Hence you would say isPrototype(thing) != FalseTriState. But this would require moving the declaration of TriState from WebCore into WTF, so that'll be a separate patch. This TriState approach is great because I think it more accurately captures other forms of queries in JSC. It's common for us to be able to answer a question with "maybe". > > 2) Should be a const member function.
Geoffrey Garen
Comment 11 2013-02-01 07:04:13 PST
> This TriState approach is great because I think it more accurately captures other forms of queries in JSC. It's common for us to be able to answer a question with "maybe". I did this in bug 108628. > I’m not really sure what a conservative estimate means. Added a comment to try to clarify this. > 2) Should be a const member function. Fixed in bug 108628.
Note You need to log in before you can comment on or make changes to this bug.