Closed Bug 388422 Opened 13 years ago Closed 11 years ago

Avoid checks for editability in BindToTree

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: peterv, Assigned: smaug)

References

(Depends on 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

UpdateEditableState shows up in the profile for bug 388108. Should find a way to avoid it unless we're sure it is needed or make it cheap (avoid checking for the contentEditable attribute for example).
To be exact, of the 14482 hits under nsGenericElement::BindToTree, 5609 are under nsGenericHTMLElement::UpdateEditableState.

That said, there are 26671 hits under nsGenericHTMLElement::BindToTree.  Of those, for example, 5776 are under nsAttrAndChildArray::SetMappedAttrStyleSheet.  So the UpdateEditableState cost is not any more than that.  Then again, maybe we can fix that too...

Also for comparison, the total number of hits under nsGenericElement::doInsertChildAt is 72343; 14547 of those are under nsAttrAndChildArray::InsertChildAt.
Depends on: 467422
When profiling artificial testcase
https://bugzilla.mozilla.org/attachment.cgi?id=350841, checking contentEditable
attribute takes more than 5%. We could easily fix that by using some flag to check if the attribute is there.
This is similar to ID, class and style attribute optimizations.
Assignee: peterv → Olli.Pettay
Attachment #350849 - Flags: superreview?(peterv)
Attachment #350849 - Flags: review?(peterv)
The patch does still apply cleanly to trunk.
Attached patch up-to-dateSplinter Review
Attachment #350849 - Attachment is obsolete: true
Attachment #368226 - Flags: superreview?(peterv)
Attachment #368226 - Flags: review?(peterv)
Attachment #350849 - Flags: superreview?(peterv)
Attachment #350849 - Flags: review?(peterv)
Would it make sense to have a non-virtual UpdateEditableState in nsIContent and move GetContentEditableValue to nsIContent?
Attachment #368226 - Flags: superreview?(peterv)
Attachment #368226 - Flags: superreview+
Attachment #368226 - Flags: review?(peterv)
Attachment #368226 - Flags: review+
Comment on attachment 368226 [details] [diff] [review]
up-to-date

I forgot about form controls and UpdateEditableFormControlState :-(.
http://hg.mozilla.org/mozilla-central/rev/622d0a6bfdaf
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.