Closed Bug 242039 Opened 17 years ago Closed 13 years ago

nsJSID::Equals performance issue

Categories

(Core :: XPConnect, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 410250

People

(Reporter: bzbarsky, Unassigned)

References

Details

(Keywords: perf)

I was looking at a profile that involved QIs of JS components, and it looked
like nsJSID::Equals was taking up some rather unconsciable amount of time. 
Indeed, this function gets the of "other" and compares it to mID.  The problem
is that nsJSID::GetId() actually allocates a brand new nsID object (so we have
to free it too).

nsIJSID is a scriptable interface, so we can't really deCOM it.  But we could
add a noscript IDEquals() method that takes an nsID and returns true or false
and use it here... That would at least allow us to not have to allocate and
deallocate IDs.  Or maybe have a "shared" getter that doesn't allocate like
nsIAtom has for the underlying string.

Maybe there are other possibilities here too, not sure.
Keywords: perf
> this function gets the of "other"

I meant 'gets the ID of "other".'
No, it gets worse.
[noscript] readonly attribute nsIDPtr id;
technically returns an allocated *pointer* to an non-allocated id. Which is
really silly.

The method signature *should* be [noscript, shared] readonly attribute nsID id;
(or whatever syntax of IDL attributes makes that work).
I don't think shared is the way to go. From xpidl code:

         * Confirm that [shared] attributes are only used with string, wstring,
         * or native (but not nsid, domstring, utf8string, cstring or astring) 
         * and can't be used with [array].
I'm also a little queezy about touching an interface that hasn't been touched in
over 3 years. Yeah, it's not frozen, but. What's everyone else's opinion on
adding a method to such an interface?
Lasty can you give a test scenario that causes a lot of QI's to JS components? I
could create a test case, but I'd rather find a real world situation to see how
much this affects things.
Oh, sorry.  As things stand, the testcases in bug 242036 will give you oodles of
QIs on a JS component.  I was going to mark the dependency but forgot.

They're pretty real-world; I took a DHTML game and stipped out the game part.  ;)

It's not a huge issue there; it's overwhelmed by all the other mess that's going
on.  But it was one of the few areas showing up in the profile where there
looked to be an easy win.
Blocks: 242036
As long as we can add the method w/o breaking the signature of the existing
interface or any interface that inherits it, I'd say it's fine to add to the
interface.
Assignee: dbradley → nobody
QA Contact: pschwartau → xpconnect
duping to bug 410250, which has a patch to take care of this problem, among others.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 410250
You need to log in before you can comment on or make changes to this bug.