1681003 - Figure out the root cause of bug 1671170

Status: RESOLVED FIXED

(Core :: Memory Allocator, task)

Description

[Emilio Cobos Álvarez (:emilio)]

That is, the cause of https://github.com/servo/rust-smallvec/issues/243. I can confirm that:

• The regression is still present on smallvec 1.5.1: https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=0abf0aa8e985149d8eaa7859a3db7d5e1495ecd1&newProject=try&newRevision=0dcc86dec1f6eae6b467974cf516d65d33e301f3&framework=10
• The regression appears in https://github.com/servo/rust-smallvec/commit/bdfc429: https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=470cb1b8a818c20b9838503068fa0e8ef0ac44ee&newProject=try&newRevision=a1232b0cee559ba85b65cdaee8779b16e95523ad&framework=10
• The regression seems, as expected, gone if I revert that commit on 1.5.1: https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=0abf0aa8e985149d8eaa7859a3db7d5e1495ecd1&newProject=try&newRevision=db644c963ebb441d38bb3d514f35f39c39699b3d&framework=10

So somehow the change from malloc + memcpy to realloc made it slower, which is unexpected. My guess is that something is holding on a global lock when it shouldn't in realloc, but that doesn't happen on malloc somehow...

The fact that it was a Windows + Linux regression, but not Mac, makes me suspect PHC is somehow involved (PHC is not enabled by default on Mac). This will eventually compare trunk vs. 1.5.1 with PHC disabled:

• https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=627f5047624a16e2d7e4148d6a0275b84c3a9235&newProject=try&newRevision=a52f4cb11014782dc1ba40031ea8c59f6770d3f8&framework=10

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Preliminary investigation does look like this is a PHC / jemalloc issue.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

So if I understand correctly, PageAlloc has a mechanism to avoid multiple threads racing for an allocation doing slow stuff like getting stack-traces by incrementing the delay, but that's not done by PageRealloc. And every time you realloc something that PHC allocated, we get a new stack using the global mutex...

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Tentative patch: https://hg.mozilla.org/try/rev/b2766055dbc8e5ad8edae87310ec5498f81873fd

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Nope, so regression is still there with PHC disabled, and thus my patch does ~nothing and my hypothesis was wrong.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Ok, second hypothesis: RallocGrowLarge is slow.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Ok, here's a better theory. This call should actually switch arenas for larger allocations. Otherwise we grow the thread-local arenas a lot.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1681003 - When realloc() grows an allocation, allow switching out of public arenas if plain malloc() would've done that. r=glandium

Comment 8

[Emilio Cobos Álvarez (:emilio)]

The regression seems gone with that patch: https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=0abf0aa8e985149d8eaa7859a3db7d5e1495ecd1&newProject=try&newRevision=a64a8412d04de4aee3d807343b928663a213562f

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ac54ee8d90e5
When realloc() grows an allocation, allow switching out of public arenas if plain malloc() would've done that. r=glandium

Comment 10

[Andreea Pavel [:apavel]]

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