Closed Bug 1598327 Opened 10 months ago Closed 10 months ago

The constructor of `AutoStyleCacheArray` appears in profile for bug 1346723

Categories

(Core :: DOM: Editor, defect, P3)

70 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
thunderbird_esr60 --- unaffected
thunderbird_esr68 --- unaffected
firefox-esr68 --- unaffected
firefox70 --- wontfix
firefox71 --- wontfix
firefox72 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: perf, regression)

Attachments

(2 files)

No description provided.

Calling AppendElement() a lot causes the constructor appearing in profile.
AutoTArray has a constructor taking initialization list. Let's use it.

The initialization cost of AutoStyleCacheArray is still expensive and it's
used only by HTMLEditor. Therefore, we should make it Maybe and construct
it only when the editor is an HTMLEditor.

Depends on D54253

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/fd1c9515bd8b
part 1: Make `AutoStyleCacheArray` initialize itself with initialization list r=m_kato
https://hg.mozilla.org/integration/autoland/rev/c2996e694ada
part 2: Make `TopLevelEditSubActionData::mCachedInlineStyle` create only in `HTMLEditor` r=m_kato
Backout by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/71513d68812b
Backed out 2 changesets for browser chrome failures on browser_bug399606.js

Really? That's odd to affect only one browser chrome test even if the patches were wrong...

Flags: needinfo?(masayuki)

It seems that it's a symptom of bug 1583364 and there is at least one buggy browser-chrome test which affects following tests. After it's backed out, I cannot reproduce it on tryserver. I'll reland the patches as-is.

Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/a2110a152aa5
part 1: Make `AutoStyleCacheArray` initialize itself with initialization list r=m_kato
https://hg.mozilla.org/integration/autoland/rev/4a55bfa7885f
part 2: Make `TopLevelEditSubActionData::mCachedInlineStyle` create only in `HTMLEditor` r=m_kato
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #2)

Created attachment 9110758 [details]
Bug 1598327 - part 2: Make TopLevelEditSubActionData::mCachedInlineStyle create only in HTMLEditor r=m_kato!

The initialization cost of AutoStyleCacheArray is still expensive and it's
used only by HTMLEditor. Therefore, we should make it Maybe and construct
it only when the editor is an HTMLEditor.

Depends on D54253

FWIW, I think part of the pain here is that constructing AutoStyleCacheArray requires constructing a bunch of StyleCache elements prior to moving them into the array. All those temporary StyleCache elements need to be destroyed, including all of the nsString instances in them (19 destructor calls that are completely useless). So maybe we could use EmplaceBack to eliminate those temporary objects, along the lines of https://phabricator.services.mozilla.com/D54253#1650797 ? Or if AutoStyleCacheArray is redesigned, it could construct fewer nsString temporaries.

You need to log in before you can comment on or make changes to this bug.