Closed
Bug 1386381
Opened 7 years ago
Closed 7 years ago
nsTextFragment::SetTo() could be faster by allocating less often
Categories
(Core :: DOM: Core & HTML, enhancement, P2)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: ehsan.akhgari, Assigned: m_kato)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
1.26 KB,
patch
|
Details | Diff | Splinter Review | |
1.53 KB,
patch
|
m_kato
:
review+
|
Details | Diff | Splinter Review |
This function currently allocates memory extremely inefficiently. Here are some examples:
* It allocates memory even if the requested length in the same type of buffer already allocated is less than what we already have allocated.
* It doesn't have nsTArray-like exponential growth with a cap behavior, so in cases where it's called in a loop it ends up allocating/copying too much, and can end up having quadratic behavior.
* For performance reasons, it may be worth considering keeping the fragment wide once it is allocated as wide instead of freeing it and reallocating it as narrow if SetTo() is called later with a narrow string.
Updated•7 years ago
|
Priority: -- → P1
Reporter | ||
Comment 1•7 years ago
|
||
Note that this is a super small part of the profiles currently. I'm keeping the bug open hoping that after we shave off enough of the cost from other places here, this starts to become a significant part of the cost in profiles...
Assignee | ||
Comment 4•7 years ago
|
||
Assignee | ||
Comment 5•7 years ago
|
||
bug 1375910's micro benchmark can improve 10%+ by WIP.
Comment 6•7 years ago
|
||
Just modified the WIP a bit to ensure we don't keep too large string buffers alive.
remote: View your change here:
remote: https://hg.mozilla.org/try/rev/fa39c24cb00c7c0bc8ddcc298f7daafe80a1c6a5
remote:
remote: Follow the progress of your build on Treeherder:
remote: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa39c24cb00c7c0bc8ddcc298f7daafe80a1c6a5
remote: recorded changegroup in replication log in 0.064s
Attachment #8940695 -
Flags: review?(m_kato)
Assignee | ||
Updated•7 years ago
|
Attachment #8940695 -
Flags: review?(m_kato) → review+
Updated•7 years ago
|
Assignee: nobody → m_kato
Pushed by opettay@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a0cf29a08c3d
nsTextFragment::SetTo() could be faster by allocating less often, p=makoto,smaug, r=makoto,smaug
Comment 8•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•