Open Bug 694034 Opened 9 years ago Updated 2 years ago

Add Telemetry probes for innerHTML

Categories

(Core :: DOM: Core & HTML, defect, P5)

defect

Tracking

()

People

(Reporter: smaug, Unassigned)

Details

Attachments

(1 file, 1 obsolete file)

We could try to collect how often innerHTML is used just to set some
short string. If that is very common, we should try to fix the existing optimizations bugs we have related to that.
OS: Linux → All
Hardware: x86 → All
Version: unspecified → Trunk
Asking strictly as a DOM newbie, is there a single place that innerHTML sets go through?  Are you envisioning gathering a simple distribution of lengths?  If the answer to both of those is yes, that doesn't sound too hard to implement.
http://mxr.mozilla.org/mozilla-central/search?string=setinnerhtml shows there are three places. nsGenericHTMLElement::SetInnerHTML is the most relevant one that covers the majority of the cases.
Thanks for the pointer.  I think this is about what you want.
Attachment #567476 - Flags: review?(Olli.Pettay)
Comment on attachment 567476 [details] [diff] [review]
add Telemetry probes for setting innerHTML

I think I'd like to see how often setting innerHTML ends up creating 
just one text node. And even better would be to record if before
setting innerHTML there was just one text node.

(We need to test how must just having the probes there slow down
innerHTML.)
Attachment #567476 - Flags: review?(Olli.Pettay) → review-
OK, here's a different patch that just handles the nsGenericHTMLElement path.  I know nothing about content, but this seems to produce interesting results.  Light browsing indicates that we rarely have a single text node before or after...but that could just be due to bugs in the implementation.
Attachment #567476 - Attachment is obsolete: true
Bugs in what implementation?

Don't use GetChildAt(0) but GetFirstChild(). Also, if telemetry is disabled we should make sure that
IsNodeOfType doesn't run. It is a virtual method (read "slow").

But let me think still a bit if we could make the probes even more useful...
At least, it would be interesting to know if we have a text node which we're replacing with a new
text node.
I meant bugs in my implementation.  Your comment makes it sounds like I was on the right track, though.
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.
Priority: -- → P5
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.