Closed
Bug 1375331
Opened 7 years ago
Closed 7 years ago
nsPresArena::Allocate calls free() a lot
Categories
(Core :: Layout, defect, P3)
Core
Layout
Tracking
()
People
(Reporter: MatsPalmgren_bugz, Assigned: MatsPalmgren_bugz)
References
(Blocks 1 open bug)
Details
(Keywords: perf, regression)
Attachments
(1 file)
I'm looking at a perf log where I see lot's of calls to free() on this path: free() nsTArray::ShrinkCapacity() nsPresArena::Allocate() I'm pretty sure it comes from removing an entry from the FreeList::mEntries: http://searchfox.org/mozilla-central/rev/ae94cfb36d804727dfb38f2750bfcaab4621df6e/layout/base/nsPresArena.cpp#120 nsTArray::RemoveElementAt calls ShiftData which calls ShrinkCapacity: http://searchfox.org/mozilla-central/rev/ae94cfb36d804727dfb38f2750bfcaab4621df6e/xpcom/ds/nsTArray-inl.h#261 *currently* only if it becomes empty, but I don't think we should count on that. Anyway, I don't see a reason to free the array buffer even if it becomes empty because it will surely be non-empty very soon again. WIP: https://hg.mozilla.org/try/rev/ed2cc52291cc7dc817d17f0044247900c8e56e66 https://treeherder.mozilla.org/#/jobs?repo=try&revision=f0c5730da03f73f0d3bf34e4ec8aa664c07d75d5 (bug 497495 is where we started using nsTArray for this, AFAICT)
Updated•7 years ago
|
status-firefox56:
--- → affected
Assignee | ||
Comment 2•7 years ago
|
||
Legend: calls: the number of calls to nsPresArena::Allocate where we end up taking an object off a free list (i.e. the number of times we hit the changed code) large: same as calls, but where "list->mEntries.Capacity() > 500" is true (which corresponds to a malloc size of 4096 bytes or more) arenas: the number of alive arenas at the end of the test time free: the number of times free() called from here overhead: the number of allocated bytes in free list arrays that aren't used histogram: occurences of transitions to empty per malloc size ======================================== https://slashdot.org before: calls=22015 arenas=24 free=2701 overhead=9600 bytes after: large=0 arenas=19 free=0 overhead=14784 bytes histogram[16]: 503 histogram[32]: 427 histogram[64]: 341 histogram[128]: 122 histogram[256]: 397 histogram[512]: 153 histogram[1024]: 24 histogram[2048]: 81 http://cnn.com/ before: calls=20891 arenas=8 free=1598 overhead=9896 bytes after: large=4199 arenas=8 free=3 overhead=13424 bytes histogram[16]: 319 histogram[32]: 574 histogram[64]: 178 histogram[128]: 86 histogram[256]: 205 histogram[512]: 99 histogram[1024]: 155 histogram[2048]: 9 histogram[4096]: 3 https://www.youtube.com/ before: calls=21958 arenas=6 free=842 overhead=16504 bytes after: large=12434 arenas=5 free=7 overhead=28248 bytes histogram[16]: 153 histogram[32]: 108 histogram[64]: 29 histogram[128]: 164 histogram[256]: 48 histogram[512]: 263 histogram[1024]: 1 histogram[2048]: 17 histogram[4096]: 2 histogram[8192]: 2 histogram[16384]: 1 histogram[32768]: 2 https://en.wikipedia.org/ before: calls=5724 arenas=11 free=1084 overhead=8064 bytes after: large=77 arenas=11 free=0 overhead=9280 bytes histogram[16]: 257 histogram[32]: 235 histogram[64]: 181 histogram[128]: 141 histogram[256]: 273 histogram[512]: 4 The test is: load the URL using a fresh profile and wait 1 minute 15 seconds, then measure the overhead over all arenas. (Using "firefox -layoutdebug" (at 800x1000) to minimize UI overhead) ======================================== Conclusion: the overhead increases but the numbers are fairly small. In exchange, we almost eradicate the number of malloc/free calls associated with these arrays. Note 1: we should save at least the same amount of malloc calls as the "free" numbers above Note 2: the difference in number of arenas before/after is likely due to ads and other dynamic effects, so all numbers are ball park figures
Assignee | ||
Comment 3•7 years ago
|
||
FTR, resizing the window tilts the histogram even more towards lower sizes.
Assignee | ||
Comment 4•7 years ago
|
||
(I'm removing the "LIFO behavior for best cache utilization" because I think it's obsolete - it's about the linked list implementation we had before bug 497495.)
Attachment #8880974 -
Flags: review?(dbaron)
Attachment #8880974 -
Flags: review?(dbaron) → review+
Pushed by mpalmgren@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e4aad4b94ca6 Retain the array buffer for PresArena free lists if it's 2048 bytes or less to avoid malloc/free churn. r=dbaron
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e4aad4b94ca6
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Updated•7 years ago
|
Updated•2 years ago
|
Performance Impact: --- → ?
Whiteboard: [qf]
You need to log in
before you can comment on or make changes to this bug.
Description
•