1363959 - Consider atomizing the strings for the comparisons while resolving a property

Status: RESOLVED FIXED

(Core :: XPConnect, defect, P2)

Description

[Ting-Yu Chou [:ting] (away)]

From the profile of test case attachment 8848224 [details], I see string comparisons:

  js::StringEqualsAscii() 
    xpc::TryResolvePropertyFromSpecs() [1],
    mozilla::dom::XrayResolveOwnProperty() [2]

  strcmp()
    xpc::XrayTraits::getExpandoObject() [3]
    
I wonder if they worth atomizing, so the string comparison can be a simple pointer check.

[1] http://searchfox.org/mozilla-central/rev/d66b9f27d5630a90b2fce4d70d4e9050f43df9b4/js/xpconnect/wrappers/XrayWrapper.cpp#424,448
[2] http://searchfox.org/mozilla-central/rev/d66b9f27d5630a90b2fce4d70d4e9050f43df9b4/dom/bindings/BindingUtils.cpp#1689,1719
[3] http://searchfox.org/mozilla-central/rev/d66b9f27d5630a90b2fce4d70d4e9050f43df9b4/js/xpconnect/wrappers/XrayWrapper.cpp#1152

Comment 1

[Ting-Yu Chou [:ting] (away)]

(In reply to Ting-Yu Chou [:ting] from comment #0)
> I wonder if they worth atomizing, so the string comparison can be a simple
> pointer check.

I meant I wonder if they can be atomized...

Comment 2

[Ting-Yu Chou [:ting] (away)]

(In reply to Ting-Yu Chou [:ting] from comment #0)
>   strcmp()
>     xpc::XrayTraits::getExpandoObject() [3]

Bug 1363963 ease this one.

Comment 3

[Ting-Yu Chou [:ting] (away)]

Profile shows this doesn't really worth P1 after bug 1363963 landed.

Comment 4

[Ting-Yu Chou [:ting] (away)]

Attachment: patch v1

Comment 5

[Ting-Yu Chou [:ting] (away)]

(In reply to Ting-Yu Chou [:ting] from comment #0)
> From the profile of test case attachment 8848224 [details], I see string
> comparisons:
> 
>   js::StringEqualsAscii() 
>     xpc::TryResolvePropertyFromSpecs() [1],
>     mozilla::dom::XrayResolveOwnProperty() [2]

I can still see [2] in VTune, so the patch removes it.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/861e1a6afff7
Compare jsid equality instead of string comparison in XrayResolveOwnProperty(). r=bholley

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/861e1a6afff7