Closed Bug 242039 Opened 17 years ago Closed 13 years ago
JSID::Equals performance issue
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.
> 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.
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.
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.