1375331 - nsPresArena::Allocate calls free() a lot

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Mats Palmgren (inactive)]

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)

Comment 1

[(no longer active)]

Shows up in Speedometer.

Comment 2

[Mats Palmgren (inactive)]

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

Comment 3

[Mats Palmgren (inactive)]

FTR, resizing the window tilts the histogram even more towards
lower sizes.

Comment 4

[Mats Palmgren (inactive)]

Attachment: Retain the array buffer for PresArena free lists if it's 2048 bytes or less to avoid malloc/free churn

(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.)

Comment 5

[Pulsebot]

Pushed by [e-mail]:
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

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e4aad4b94ca6