Track inheritance structures in a side table, instead of using a private name in each prototype

• Component: New Bugs
• Status: RESOLVED FIXED

[Geoffrey Garen]

Track inheritance structures in a side table, instead of using a private name in each prototype

[Geoffrey Garen]

Created attachment 183632 [details]
Patch

[Geoffrey Garen]

Created attachment 183633 [details]
performance

[Geoffrey Garen]

Seems performance-neutral, as expected.

[Geoffrey Garen]

Comment on attachment 183632 [details]
Patch

Sam and Phil reviewed this.

[Geoffrey Garen]

Committed r140259: <http://trac.webkit.org/changeset/140259>

[Geoffrey Garen]

ASSERTing on a sputnik test: http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK1%20(Tests)/r140274%20(4771)/sputnik/Conformance/13_Function_Definition/13.2_Creating_Function_Objects/S13.2.2_A3_T1-crash-log.txt.

[Geoffrey Garen]

Disabled the ASSERT for the time being to get the bots green: <http://trac.webkit.org/changeset/140259>.

[Geoffrey Garen]

Follow-up ASSERT fix: <http://trac.webkit.org/changeset/140284>.

[Darin Adler]

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]

(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]

> 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.