Closed Bug 1375331 Opened 3 years ago Closed 3 years ago

nsPresArena::Allocate calls free() a lot

Categories

(Core :: Layout, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox-esr52 --- wontfix
firefox54 --- wontfix
firefox55 --- wontfix
firefox56 --- fixed

People

(Reporter: mats, Assigned: mats)

References

(Blocks 1 open bug)

Details

(Keywords: perf, regression, Whiteboard: [qf])

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)
Shows up in Speedometer.
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
FTR, resizing the window tilts the histogram even more towards
lower sizes.
(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
https://hg.mozilla.org/mozilla-central/rev/e4aad4b94ca6
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.