395256 - Make nsJSID/IID/CID objects .equals method slightly faster

Status: RESOLVED WONTFIX

(Core :: XPConnect, defect)

Description

[Benjamin Smedberg]

Attachment: Make JSIDs faster, probably, rev. 1

This patch changes the signature of the (noscript, mainly internal) nsIJSID.equals method to take a nsIDRef instead of a nsIJSID... I suspect that this will improve performance in a few cases. There are some additional optimizations that could be made, such as keeping the IID on nsJSIID instead of forwarding requests to nsIInterfaceInfo... or making nsIJSID.id a "shared" property, except I don't think that will work for threadsafety reasons.

Comment 1

[Damon Sicore (:damons)]

Comment on attachment 279952 [details] [diff] [review]
Make JSIDs faster, probably, rev. 1

This will be approved for landing in M8, during the freeze, only once jst has SR'd.

Comment 2

[Johnny Stenback (:jst)]

Comment on attachment 279952 [details] [diff] [review]
Make JSIDs faster, probably, rev. 1

Oops, re-requesting approval as I'm not supposed to do them any more :)

Comment 3

[Benjamin Smedberg]

First patch landed on trunk. Didn't help much, so I'm going to try a patch to avoid the alloc/release.

Comment 4

[Dave Townsend [:mossop]]

This appears to have cause a regression for at least one extension in bug 395256. Given the closeness to M8 I would suggest we might want to back this out a.s.a.p. unless we have an understanding of what is going on.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

I backed out the first patch because of the regression.
mozilla/js/src/xpconnect/idl/xpcjsid.idl 	1.22
mozilla/js/src/xpconnect/src/xpcjsid.cpp 	1.72

Comment 6

[jag (Peter Annema)]

Comment on attachment 279952 [details] [diff] [review]
Make JSIDs faster, probably, rev. 1

>Index: js/src/xpconnect/idl/xpcjsid.idl
>===================================================================
> [scriptable, uuid(C86AE131-D101-11d2-9841-006008962422)]
> interface nsIJSID : nsISupports
...
>-    boolean equals(in nsIJSID other);
>+    boolean equals(in nsIDRef other);

Why was the uuid not updated for this change?

Comment 7

[Benjamin Smedberg]

Going to mark this WONTFIX... it didn't really improve perf when it landed, and I'm not sure this is a critical codepath for anything right now.