The constructor of `AutoStyleCacheArray` appears in profile for bug 1346723
Categories
(Core :: DOM: Editor, defect, P3)
Tracking
()
| 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)
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Comment 1•5 years ago
|
||
Calling AppendElement() a lot causes the constructor appearing in profile.
AutoTArray has a constructor taking initialization list. Let's use it.
|
|
Assignee | |
Comment 2•5 years ago
|
||
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
|
||
Comment 5•5 years ago
|
||
Push with failures (original push had build bustages): https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=success%2Cpending%2Crunning%2Ctestfailed%2Cbusted%2Cexception&fromchange=74f97d22bea2684f9c677f9cb81d092c5535b6f4&searchStr=browser%2Cchrome&selectedJob=277763293
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=277763293&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=277762927&repo=autoland
Backout link: https://hg.mozilla.org/integration/autoland/rev/71513d68812b4eec72883306a1aa0973f2f5951c
|
|
Assignee | |
Comment 6•5 years ago
|
||
Really? That's odd to affect only one browser chrome test even if the patches were wrong...
|
|
Assignee | |
Comment 7•5 years ago
|
||
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.
|
||
Comment 9•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/a2110a152aa5
https://hg.mozilla.org/mozilla-central/rev/4a55bfa7885f
|
||
Comment 10•5 years ago
|
||
(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #2)
Created attachment 9110758 [details]
Bug 1598327 - part 2: MakeTopLevelEditSubActionData::mCachedInlineStylecreate only inHTMLEditorr=m_kato!The initialization cost of
AutoStyleCacheArrayis still expensive and it's
used only byHTMLEditor. Therefore, we should make itMaybeand construct
it only when the editor is anHTMLEditor.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.
|
||
Updated•3 years ago
|
Description
•