nsJSID::Equals performance issue

RESOLVED DUPLICATE of bug 410250

Status

()

Core
XPConnect
RESOLVED DUPLICATE of bug 410250
14 years ago
10 years ago

People

(Reporter: bz, Unassigned)

Tracking

({perf})

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

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

Comment 2

14 years ago
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).

Comment 3

14 years ago
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].

Comment 4

14 years ago
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?

Comment 5

14 years ago
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

Comment 8

10 years ago
duping to bug 410250, which has a patch to take care of this problem, among others.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 410250
You need to log in before you can comment on or make changes to this bug.