Closed
Bug 297145
Opened 19 years ago
Closed 7 years ago
investigate whether JS_GetProperty in nsDOMClassInfo::PostCreate is needed
Categories
(Core :: DOM: Core & HTML, defect, P3)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: bryner, Assigned: jst)
References
()
Details
(Keywords: helpwanted, perf)
jst and I are a bit baffled by what this code is trying to do. If we could get
rid of it, I think it would speed up DOM traversal from script a bit.
Comment 1•19 years ago
|
||
So this looks like it's just checking that window.Node (or equivalent for other
classes) is defined, right? That does seem pretty pointless to me.... unless
this triggers our lazy global object class init?
Comment 2•19 years ago
|
||
In particular, see the work done in nsWindowSH::GlobalResolve... I guess the
intent here could be that the Node class (say) is all correctly set up before we
return a Node wrapper... If so, maybe there's a better way to do it? Say
keeping some out-of-band state as to which classes are already ok?
Comment 3•19 years ago
|
||
::JS_LookupProperty suffices to trigger a resolve hook. ::JS_GetProperty does a
lookup and if that succeeds, calls the property's getter -- which is not certain
to be idempotent. Don't do unnecessary gets when lookups are enough.
/be
Comment 4•19 years ago
|
||
I suspect the slow part here is getting the global object and making the call at
all, not the specifics of what call we're making...
Comment 5•19 years ago
|
||
This code just got removd in bug 326497, but I'm still not sure whether that's ok (see comment 2).
Flags: blocking1.9a1?
Comment 6•19 years ago
|
||
So, it turns out that the GetProperty is needed for reflecting functions stuck on <classname>.prototype.
Comment 7•19 years ago
|
||
Do we know why?
Comment 9•19 years ago
|
||
(In reply to comment #2)
> In particular, see the work done in nsWindowSH::GlobalResolve... I guess the
> intent here could be that the Node class (say) is all correctly set up before we
> return a Node wrapper...
See bug 326497 comment 11. The GetProperty is there precisely to make sure that the prototype chain is set up and delegation works. In that bug, I changed the GetProperty to a LookupProperty to avoid calling the getter, but I think the real fix is to call nsWindowSH::GlobalResolve directly from nsDOMClassInfo::PostCreate, so we'd avoid hitting the JS engine at all.
Updated•18 years ago
|
Flags: blocking1.9a1? → blocking1.9-
Updated•18 years ago
|
Assignee: jst → mrbkap
Comment 10•17 years ago
|
||
Bkap is this something we still want to look at?
Comment 11•17 years ago
|
||
Yeah, we can probably avoid doing some work by fixing this bug.
Updated•17 years ago
|
Flags: blocking1.9- → blocking1.9?
Updated•17 years ago
|
Priority: -- → P3
Assignee | ||
Comment 14•17 years ago
|
||
I've been looking for this in performance profiles lately and it's really not anything worth changing for performance reasons any more. The most time I've seen us spend in nsDOMClassInfo::PostCreate() is < 0.2% of all the time spent in JS (during startup and other JS performance tests). IMO it's simply not worth it, at least not for 1.9. Making blocking-, holler if you disagree and have a testcase that shows where this still hurts us.
Flags: blocking1.9+ → blocking1.9-
Updated•15 years ago
|
QA Contact: ian → general
Comment 15•7 years ago
|
||
Investigation complete. This code is all gone. ;)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•