Bug 1364359 is to fix a leaked arena. Until that is fixed; it is unsafe to
call moz_dispose_arena more than once.
MozReview-Commit-ID: KIby1RLtrPK
--HG--
extra : rebase_source : 6ea41001e9f0c4d5eb24ee678d6c1c0218991ac3
This behavior matches what gets used in mozalloc.h to define these
wrappers, and is particularly necessary for newer versions of clang to
not complain about our definitions.
Base allocator commit stats were added in bug 515556, along other commit
stats, but they have actually been wrong since then: the committed count
is updated with the difference between pbase_next_addr and
base_next_decommitted *after* the latter is set to the former, making
the difference always 0.
--HG--
extra : rebase_source : a2aed523314549a37a61bd4ab300c98f198f9252
Since TreeNode::{Left,Right,Color} is always a valid call to make, we
don't need to check if for nullity before calling those functions.
This effectively kind of reverts some parts of bug 1412722.
--HG--
extra : rebase_source : 3deb316f463b51fdbb3aebc2e57e437018b3a829
The code before bug 1412722 benefitted from the sentinel being an actual
node object, and some code paths ended up checking its color (always
black) or getting its right and left node, that always returned to the
sentinel.
When TreeNode currently contains a nullptr, all those lead to a null
deref if the calling code is not doing the right checks, which happens
to be the case in at least some places. Instead of relying on the
callers doing the right thing, make the TreeNode do the right thing when
it contains a nullptr, effectively acting as the sentinel in that case.
We additionally ensure that nothing in the calling code will be trying
to change the color or left/right pointers on the sentinel, which is an
extra safe net compared to the code before bug 1412722.
--HG--
extra : rebase_source : 09ab0bf8682092ef6d9a0a5921be3da787d0d548
This will allow the upcoming changes to add some safety back to the code
after bug 1412722.
--HG--
extra : rebase_source : 5033b8034cabaf5a7fdd578459588d5099402d02
Since TreeNode::{Left,Right,Color} is always a valid call to make, we
don't need to check if for nullity before calling those functions.
This effectively kind of reverts some parts of bug 1412722.
--HG--
extra : rebase_source : 172f1c042bdbb4d500e1afb4d57774ab76826876
The code before bug 1412722 benefitted from the sentinel being an actual
node object, and some code paths ended up checking its color (always
black) or getting its right and left node, that always returned to the
sentinel.
When TreeNode currently contains a nullptr, all those lead to a null
deref if the calling code is not doing the right checks, which happens
to be the case in at least some places. Instead of relying on the
callers doing the right thing, make the TreeNode do the right thing when
it contains a nullptr, effectively acting as the sentinel in that case.
We additionally ensure that nothing in the calling code will be trying
to change the color or left/right pointers on the sentinel, which is an
extra safe net compared to the code before bug 1412722.
--HG--
extra : rebase_source : ac61ea259ac49bf76e2f8f6f54dda991498d4664
This will allow the upcoming changes to add some safety back to the code
after bug 1412722.
--HG--
extra : rebase_source : c906e9b3168fe738cba8a3de3fdf4efee1f0d4df
- Turn MOZ_DIAGNOSTIC_ASSERTs related to double-frees into
MOZ_RELEASE_ASSERTs with a crash message making them more identifiable
than the asserted condition.
- In huge_dalloc, MOZ_RELEASE_ASSERT early, instead of letting
RedBlackTree::Remove end up crashing because the node is not in the
tree.
--HG--
extra : rebase_source : e051caaf371e88a9db6b5153f58c8a4aa4cde757
Before bug 1412722, which removed the sentinels, the code looked like:
if (rbp_r_c->Right()->Left()->IsBlack()) {
At that point in the code, rbp_r_c is the root node of the tree. If
rbp_r_c->Right() was the sentinel, ->Right()->Left() would be the
sentinel too, and the sentinel is black. Which means the condition would
be true.
The code after was:
if (rbp_r_c->Right() && (!rbp_r_c->Right()->Left() ||
rbp_r_c->Right()->Left()->IsBlack())) {
The second half correctly deals with the case of
rbp_r_c->Right()->Left() being the sentinel. But the first half now
makes things different: ->Right() being null would correspond to the
previous case where it was the sentinel, and the test would not return
true in that case when it would have before. When ->Right() is not null,
things are normal again.
The correct check is to make the branch taken when ->Right() is null.
Now, looking under which conditions we may get in that branch wrongly:
- The root node's right link must be empty, which means a very small tree.
- The comparison between the removed key and the root node must indicate
the key is greater than the value of the root node.
- There's another case where the comparison result (rbp_r_cmp) can be
eGreater, when it is reassigned under one of the branches under the
eEqual test, and that branch is only taken when ->Right() on the root
node was non-null, which was the non-broken case.
So it would seem we can't reach that code when rbp_r_c->Right() is null
anyways, so it /should/ practically make no difference. Better safe than
sorry, though. It's hard to tell anything from crash stats, because
since the templatization in bug 1403444, all crashes fit in one bucket,
when there used to be 5 functions before :(
While here, add a missing include in rb.h.
--HG--
extra : rebase_source : 2ebcb84345ad52059b0c081b9e2e1af1d0bbb7bc
We're not actually using it, and it messes up with the zone allocator in
mozjemalloc after fork(). See the lengthy analysis in
https://bugzilla.mozilla.org/show_bug.cgi?id=1424709#c34 and following.
--HG--
extra : rebase_source : c58e13b897dde7b32d83c43fbb2a04a0db3a5dc9
They are both infallible wrappers of posix_memalign and valloc.
There is also moz_xmemalign, which wraps memalign, which is mostly
always available as of bug 1402647.
None of them are actually used, but it's still desirable to at least
have one infallible variant, so keep moz_xmemalign and remove the other
two.
While here, we actually make both memalign and moz_xmemalign always
available.
--HG--
extra : rebase_source : 1c3ca4b3e3310543145f3181dfa4e764be1d6ff8
Most adjustements come from some recent .clang-format changes. A few
were overlooked from changes to the code.
--HG--
extra : rebase_source : c397762ea739fec05256a8c0132541bf4c727c32
Also change the definition of the StaticMutex constructor to unconfuse clang-format.
--HG--
extra : rebase_source : aa2aa07c8c324e1253c20b34d850230579d45632
And statically link logalloc.
Statically linking is the default, except when building with
--enable-project=memory, allowing to use the generated libraries from
such builds with Firefox.
--HG--
extra : rebase_source : efe9edce8db6a6264703e0105c2192edc5ca8415
The original purpose of those declarations was to avoid the function
definitions being wrong, as well as forcing them being exported
properly (as extern "C", as weak symbols when necessary, etc.), but:
- The implementations being C++, function overloads simply allowed
functions with the same name to have a different signature.
- As of bug 1420353, the functions don't need to be exported anymore,
nor do we care whether their symbols are mangled. Furthermore, they're
now being assigned to function table fields, meaning there is type
checking in place, now.
So all in all, these declarations can be removed.
Also, as further down the line we're going to statically link the
replace-malloc libraries, avoid symbol conflicts by making those
functions static.
--HG--
extra : rebase_source : 0dbb15f2c85bc873e7eb662b8d757f99b0732270
And statically link logalloc.
Statically linking is the default, except when building with
--enable-project=memory, allowing to use the generated libraries from
such builds with Firefox.
--HG--
extra : rebase_source : efe9edce8db6a6264703e0105c2192edc5ca8415
The original purpose of those declarations was to avoid the function
definitions being wrong, as well as forcing them being exported
properly (as extern "C", as weak symbols when necessary, etc.), but:
- The implementations being C++, function overloads simply allowed
functions with the same name to have a different signature.
- As of bug 1420353, the functions don't need to be exported anymore,
nor do we care whether their symbols are mangled. Furthermore, they're
now being assigned to function table fields, meaning there is type
checking in place, now.
So all in all, these declarations can be removed.
Also, as further down the line we're going to statically link the
replace-malloc libraries, avoid symbol conflicts by making those
functions static.
--HG--
extra : rebase_source : 0dbb15f2c85bc873e7eb662b8d757f99b0732270
The definition for replace_init is only used in two places:
replace_malloc.h and mozjemalloc.cpp. Invoking the malloc_decls.h
machinery for just that one function is a lot of overhead, and things
are actually simpler doing the declaration directly, especially thanks
to the use of C++11 decltype to avoid duplication of the function
type.
This has the additional advantage that now malloc_decls.h only contains
the allocator API.
While here, replace the MOZ_WIDGET_ANDROID ifdef with ANDROID.
--HG--
extra : rebase_source : 50ddf044f43904575598f17bd4c3477fc1a1155f
Because one entry point is simpler than two, we make replace_init fulfil
both the roles of replace_init and replace_get_bridge.
Note this should be binary compatible with older replace-malloc
libraries, albeit not detecting their bridge (and with the
previous change, they do not register anyways). So loading older
replace-malloc libraries should do nothing, but not crash in awful ways.
--HG--
extra : rebase_source : aaf83e706ee34f45cfa75551a2f0998e5c5b8726
The allocator API is a moving target, and every time we change it, the
surface for replace-malloc libraries grows. This causes some build
system problems, because of the tricks in replace_malloc.mk, which
require the full list of symbols.
Considering the above and the goal of moving some of the replace-malloc
libraries into mozglue, it becomes simpler to reduce the replace-malloc
exposure to the initialization functions.
So instead of the allocator poking into replace-malloc libraries for all
the functions, we expect their replace_init function to alter the table
of allocator functions it's passed to register its own functions.
This means replace-malloc implementations now need to copy the original
table, which is not a bad thing, as it allows function calls with one
level of indirection less. It also replace_init functions to not
actually register the replace-malloc functions in some cases, which will
be useful when linking some replace-malloc libraries into mozglue.
Note this is binary compatible with previously built replace-malloc
libraries, but because those libraries wouldn't update the function
table, they would stay disabled.
--HG--
extra : rebase_source : 2518f6ebe76b4c82359e98369de6a5a8c3ca9967
Some platforms (at least powerpc64) apparently can't handle long double
constants. So use double constants instead.
--HG--
extra : rebase_source : dd7f87cfff13f63566845e03a0bd6f4a8146f421
Bug 1417234 replaced all uses of CRITICAL_SECTION with SRWLocks in the
allocator on Windows, and this seems to have incurred performance
regressions on speedometer.
OTOH, there is a real benefit from not having to manually initialize the
allocator.
So we restore the use of CRITICAL_SECTIONs for Mutexes in the allocator,
except for the initialization lock, which is remaining as a SRWLock.
Talos indicates this solves the regression in large part, but is not
definitive as whether it has the same effect as a pure backout of bug
1417234. We'll see how things go over time.
--HG--
extra : rebase_source : a52b8d08159f6fce8286578114af84e55851a86b
- This makes all of them return an enum, quite similar to rust.
- This makes all of them derive from a single implementation of an
integer comparison.
- This implements that integer comparison in terms of simple operations,
rather than "smart" computation, which turns out to allow compilers
to do better optimizations.
See https://blogs.msdn.microsoft.com/oldnewthing/20171117-00/?p=97416
--HG--
extra : rebase_source : 89710d7954f445d43e6da55aaf171b1f57dce5fc
Zero and junk should have the same scope, but currently huge and large
reallocs are zeroed when zeroing is enabled, but are not junked when
junking is enabled. This makes things straight, leaning on the side of
filling the added bytes, rather than not.
This has an actual effect on debug builds, where junk is enabled by
default.
--HG--
extra : rebase_source : f409cae7ea720f69239d896d155b653efc648feb
This also creates a new arena_t::Ralloc function replacing parts of
iralloc, the other part being inlined in the unique caller.
--HG--
extra : rebase_source : 76a9ca77e651c99641d8906faea8e469d8eea041
Since the old size arena_ralloc is given is already a size class, when
the size class for the new size is the same as the old size, the new
size can't be larger than the old size, so there's never anything to
zero.
--HG--
extra : rebase_source : dd468de60b2ed87718ec4e26f13d3b0c8932f455
We intend to move some functions to methods of the arena_t class. Moving
the arena selection out of them is the first step towards that.
--HG--
extra : rebase_source : b8380c3a0c90ed817a1dbbe8d60e6107b78ec677
The immediate goal for this is to allow determinism in an arena used for
an upcoming test, by essentially disabling purge on that specific arena.
We do that by allowing arenas to be created with a specific setting for
mMaxDirty.
Incidentally, this allows to cleanup the mMaxDirty initialization for
thread-local arenas.
Longer term, this would allow to tweak arenas with more parameters, on
a per arena basis.
--HG--
extra : rebase_source : e4b844185d132aca9ee10224fc626f8293be0a34
Some unit tests rely on jemalloc_stats to get information such as chunk
size or page size. They can do so before any allocation happens, when
using gtest filters. So it is preferable for jemalloc_stats to
initialize the allocator.
--HG--
extra : rebase_source : 6696ec1cdaa3b121a3d12cb7b6049b79c656d271
SRWLock is more lightweight than CriticalSection, but is only available
on Windows Vista and more. So until we actually dropped support Windows
XP, we had to use CriticalSection.
Now that all supported Windows versions do have SRWLock, this is a
switch we can make, and not only because SRWLock is more lightweight,
but because it can be statically initialized like on other platforms,
allowing to use the same initialization code as on other platforms,
and removing the requirement for a DllMain, which in turn can allow
to statically link mozjemalloc in some cases, instead of requiring a
shared library (DllMain only works on shared libraries), or manually
call the initialization function soon enough.
There is a downside, though: SRWLock, as opposed to CriticalSection, is
not fair, meaning it can have thread scheduling implications, and can
theoretically increase latency on some threads. However, it is the
default used by Rust Mutex, meaning it's at least good enough there.
Let's see how things go with this.
--HG--
extra : rebase_source : 337dc4e245e461fd0ea23a2b6b53981346a545c6
Currently, huge allocations are completely independent from arenas. But
in order to ensure that e.g. moz_arena_realloc can't reallocate huge
allocations from another arena, we need to track which arena was
responsible for the huge allocation. We do that in the corresponding
extent_node_t.
Both functions do essentially the same thing, one having more validation
than the other. We can use a template with a boolean parameter to avoid
the duplication.
Furthermore, we're soon going to require, in some cases, more
information than just the size of the allocation, so we wrap their
result in a helper class that gives information about an active
allocation.
Masatoshi Kimura
Tom Ritter
Nathan Froyd
Mike Hommey
Andreea Pavel
Bobby Holley
Sylvestre Ledru
Csoregi Natalia