Closed
Bug 517380
Opened 15 years ago
Closed 3 years ago
JSOP_CALLNAME in an event handler seems to always miss the property cache
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
INVALID
People
(Reporter: bzbarsky, Unassigned)
References
Details
(Keywords: perf)
Attachments
(1 file)
(deleted),
text/html
|
Details |
STEPS TO REPRODUCE:
1) Breakpoint in the JSOP_CALLNAME case in jsops.cpp
2) Load attached testcase.
3) Step down to right after the PROPERTY_CACHE_TEST when trying to call doclick
4) Observer that atom != null, and is in fact "doclick".
5) Note that this happens every time we try to call doclick.
6) Profile and observe that this does a resolve on the document, which does
various DOM traversal stuff and otherwise sucks.
Reporter | ||
Comment 1•15 years ago
|
||
Reporter | ||
Comment 2•15 years ago
|
||
ccing my friendly propcache victims. ;)
Comment 3•15 years ago
|
||
One thing to try is to step into js_FullTestPropertyCache (within PROPERTY_CACHE_TEST) and see why it fails to hit.
Another route: define JS_PROPERTY_CACHE_METERING and look at /tmp/propcache.stats.
/be
Reporter | ||
Comment 4•15 years ago
|
||
In js_FindPropertyHelper, scopeChain is the <div>'s JSObject, which has a parent and is not a js_IsCacheableNonGlobalScope. So we never even try to fill the propcache in this case.
Comment 5•15 years ago
|
||
Sorry, should have seen that coming -- see bug 503694.
Fixable, but we reckoned unobtrusive JS was favored style, event handlers want to be short/concise, so we didn't need to work on optimizing deep native scope chain traces. Were we wrong?
/be
Reporter | ||
Comment 6•15 years ago
|
||
For purposes of actual web compat, probably not. For purposes of silly benchmarks that have the event handler call a function that basically does nothing? Not sure. :(
Assignee | ||
Updated•10 years ago
|
Assignee: general → nobody
Comment 7•3 years ago
|
||
Old bug, no longer valid. Any current performance issues will have other causes than the ones discussed here.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•