Commit Graph

572 Commits

Author SHA1 Message Date
Paul Bone
883afb9d00 Bug 1808429 - Disconnect runtime configuration from MOZ_DEBUG r=glandium
Some of the runtime options are compiled-in constants when MOZ_DEBUG is not
defined.  But it can be useful to enable these configuration options without
enabling the rest of MOZ_DEBUG (eg assertions).  This patch adds a new
preprocessor macro to enable runtime configuration.

Differential Revision: https://phabricator.services.mozilla.com/D165920
2023-01-20 06:21:12 +00:00
Paul Bone
95ef1322fd Bug 1808429 - Add the ability to enable and disable poisoning r=glandium
Poisoning was hardcoded, but to compare different allocators I wanted to
disable it.  This patch lets us control poisoning with the MALLOC_OPTIONS
environment variable.

Differential Revision: https://phabricator.services.mozilla.com/D165919
2023-01-20 06:21:12 +00:00
Paul Bone
4a359a003e Bug 1808429 - Fix non-existant parameters in code comment r=glandium
This comment refers to parameters that don't exist.

Differential Revision: https://phabricator.services.mozilla.com/D165918
2023-01-20 06:21:12 +00:00
Ray Kraesig
72c9c808cd Bug 1804499 - [4/4] Expose allocator's stall-specs beyond mozjemalloc.cpp r=glandium
Export `GetStallSpecs` as `mozilla::GetAllocatorStallSpecs`, when it exists.

Differential Revision: https://phabricator.services.mozilla.com/D165430
2023-01-20 01:50:54 +00:00
Ray Kraesig
4fcae818b2 Bug 1804499 - [3/4] Expose stall-and-retry code beyond mozjemalloc.cpp r=glandium
Extract the stall-and-retry logic from mozjemalloc into a header. Write
a unit test for it.

Differential Revision: https://phabricator.services.mozilla.com/D164107
2023-01-20 01:50:54 +00:00
Ray Kraesig
00c3eaa89a Bug 1804499 - [2/4] Factor out the stall logic in preparation for export r=glandium
In bug 1794059 it was noted that the IPC shared-memory allocation code
would like to be able to stall-and-retry as well using the same logic.

While it doesn't use VirtualAlloc, the principle is otherwise the same.
Shuffle the relevant code around so that the stall-and-retry logic is
separate from the allocation, in preparation for exporting it.

Differential Revision: https://phabricator.services.mozilla.com/D164106
2023-01-20 01:50:53 +00:00
Ray Kraesig
800ec51744 Bug 1804499 - [1/4] Handle Windows error code consistently r=glandium
The real ::VirtualAlloc does not modify the existing last-error code in
case of success; therefore, neither should we.

Differential Revision: https://phabricator.services.mozilla.com/D164105
2023-01-20 01:50:53 +00:00
Cosmin Sabou
2fa42b60ac Backed out 4 changesets (bug 1804499) for causing SM bustages.
Backed out changeset 54dd0be202ef (bug 1804499)
Backed out changeset 3131f330b55d (bug 1804499)
Backed out changeset 180af13cd33f (bug 1804499)
Backed out changeset a05c18f43067 (bug 1804499)
2023-01-19 18:56:08 +02:00
Ray Kraesig
611b09e06e Bug 1804499 - [4/4] Expose allocator's stall-specs beyond mozjemalloc.cpp r=glandium
Export `GetStallSpecs` as `mozilla::GetAllocatorStallSpecs`, when it exists.

Differential Revision: https://phabricator.services.mozilla.com/D165430
2023-01-19 16:16:43 +00:00
Ray Kraesig
0336c0891c Bug 1804499 - [3/4] Expose stall-and-retry code beyond mozjemalloc.cpp r=glandium
Extract the stall-and-retry logic from mozjemalloc into a header. Write
a unit test for it.

Differential Revision: https://phabricator.services.mozilla.com/D164107
2023-01-19 16:16:42 +00:00
Ray Kraesig
2696c25e5c Bug 1804499 - [2/4] Factor out the stall logic in preparation for export r=glandium
In bug 1794059 it was noted that the IPC shared-memory allocation code
would like to be able to stall-and-retry as well using the same logic.

While it doesn't use VirtualAlloc, the principle is otherwise the same.
Shuffle the relevant code around so that the stall-and-retry logic is
separate from the allocation, in preparation for exporting it.

Differential Revision: https://phabricator.services.mozilla.com/D164106
2023-01-19 16:16:42 +00:00
Ray Kraesig
a27f258c20 Bug 1804499 - [1/4] Handle Windows error code consistently r=glandium
The real ::VirtualAlloc does not modify the existing last-error code in
case of success; therefore, neither should we.

Differential Revision: https://phabricator.services.mozilla.com/D164105
2023-01-19 16:16:41 +00:00
Paul Bone
4ea7329f5a Bug 1806049 - Explicitly use uint32_t r=glandium
We want to be precise about types used here.  Although in practice unsigned
is the same as uint32_t, it's not guaranteed.  We want to definitely use
32-bit multiplication as it can be faster than 64-bit.

Differential Revision: https://phabricator.services.mozilla.com/D164889
2023-01-10 22:30:05 +00:00
Paul Bone
f9bccca030 Bug 1806049 - Test all divisions r=glandium
Don't wait until runtime to test that all valid divisions work.  Test them
during initialisation in debug builds.

Differential Revision: https://phabricator.services.mozilla.com/D164888
2023-01-10 22:30:05 +00:00
Paul Bone
bfb66a35a9 Bug 1806049 - Rename the FastDivisor variables r=glandium
Rename the `m` and `p` variables to match those used in the Hacker's Delight
book where the algorithm is presented.  There were also some inconsistent
names in comments that this fixes.

Differential Revision: https://phabricator.services.mozilla.com/D164887
2023-01-10 22:30:04 +00:00
Paul Bone
e180463f6d Bug 1806049 - Improve some code comments r=glandium
Differential Revision: https://phabricator.services.mozilla.com/D164886
2023-01-10 22:30:04 +00:00
Otto Länd
fe6866347e Bug 1743322, 1743322: apply code formatting via Lando
# ignore-this-changeset
2022-12-13 04:26:05 +00:00
Paul Bone
a1aa315882 Bug 1743322 - Make arena_bin_t 32 or 48 bytes long r=glandium
This structure is more optimal if it is somewhat aligned with the system's
cache line length (which we assume is 64 bytes but that's not always true).
This reduces the number of cache lines required to access one record on
average. On 32-bit systems we can manage 32-bytes, on 64-bit systems we can
manage 48 bytes. We do this by:

 * Make mRunSize the number of pages in a run rather than bytes, so that it can
   be stored in a single byte and save some space in bin headers.

 * Make mNumRuns a uint32_t on all platforms.

Differential Revision: https://phabricator.services.mozilla.com/D140036
2022-12-13 04:21:15 +00:00
Paul Bone
3932940e08 Bug 1743322 - Remove branches that decide how to divide r=glandium
This code path would choose from several ways about how to divide numbers.
By calculating the inverse of the divisor early we can elude all the
branches along this code path we can make it faster than the previous code
or naive division.

Differential Revision: https://phabricator.services.mozilla.com/D132322
2022-12-13 04:21:15 +00:00
Norisz Fay
70bc66e002 Backed out 3 changesets (bug 1743322) for causing multiple failures CLOSED TREE
Backed out changeset fa443c853d2d
Backed out changeset d299fa6358f7 (bug 1743322)
Backed out changeset 25647d613534 (bug 1743322)
2022-12-06 11:48:14 +02:00
Otto Länd
8b4f88bf4a No bug: apply code formatting via Lando
# ignore-this-changeset
2022-12-06 00:53:23 +00:00
Paul Bone
2423f4ad0b Bug 1743322 - Make arena_bin_t 32 or 48 bytes long r=glandium
This structure is more optimal if it is somewhat aligned with the system's
cache line length (which we assume is 64 bytes but that's not always true).
This reduces the number of cache lines required to access one record on
average. On 32-bit systems we can manage 32-bytes, on 64-bit systems we can
manage 48 bytes. We do this by:

 * Make mRunSize the number of pages in a run rather than bytes, so that it can
   be stored in a single byte and save some space in bin headers.

 * Make mNumRuns a uint32_t on all platforms.

Differential Revision: https://phabricator.services.mozilla.com/D140036
2022-12-06 00:50:51 +00:00
Paul Bone
763f464a09 Bug 1743322 - Remove branches that decide how to divide r=glandium
This code path would choose from several ways about how to divide numbers.
By calculating the inverse of the divisor early we can elude all the
branches along this code path we can make it faster than the previous code
or naive division.

Differential Revision: https://phabricator.services.mozilla.com/D132322
2022-12-06 00:50:51 +00:00
Nika Layzell
f8e46f7fb5 Bug 1797354 - Allow specifying capability type in thread-safety macros, r=jesup,media-playback-reviewers,alwu
It is possible to specify full names for capabilities when using the clang
thread-safety analysis which will be used in error messages. We should use that
form of the attribute rather than the legacy lockable attribute.

Differential Revision: https://phabricator.services.mozilla.com/D160531
2022-10-28 19:55:30 +00:00
Ray Kraesig
b90e6da27e Bug 1785162 - Allow child-process half-stalls to ride the trains r=glandium,gsvelto
The past couple of weeks of crash telemetry indicate that stalling on
OOM in child processes for half as long as in main processes drastically
reduces child process OOM crashes without noticeably increasing
main-process OOM crashes.

Give it a ticket and send it on its way.

Differential Revision: https://phabricator.services.mozilla.com/D158790
2022-10-07 15:40:24 +00:00
Ray Kraesig
310ef2663b Bug 1785162 - [3/3] stall for half time in non-main processes r=gsvelto
Retain the full stall period in the main process; stall for only half as
long in all other processes.

Differential Revision: https://phabricator.services.mozilla.com/D157140
2022-09-22 15:01:42 +00:00
Ray Kraesig
adac61618a Bug 1785162 - [2/3] refactor to specify stall count and time r=glandium,gsvelto
Refactor to allow specifying a potentially variable stall-count and
-time, rather than a binary stall/don't-stall cue.

No functional changes; this facility will actually be used in the next
commit.

Differential Revision: https://phabricator.services.mozilla.com/D157139
2022-09-22 15:01:42 +00:00
Ray Kraesig
f30c547ca5 Bug 1785162 - [1/3] Revert "Disable all-process stalling in Nightly" r=gsvelto
Sufficient data has been acquired to confirm that all-stall does indeed
increase the chance of a main-process OOM compared to just main-stall.

Revert to all-stall anyway, solely to simplify the next two commits.

Differential Revision: https://phabricator.services.mozilla.com/D157138
2022-09-22 15:01:42 +00:00
Ray Kraesig
75b7dee465 Bug 1207753 - Add TS annotations for some mozjemalloc mutexes r=jesup
Add thread-safety annotations for a few of the mutexes in mozjemalloc.

Differential Revision: https://phabricator.services.mozilla.com/D157858
2022-09-22 15:00:44 +00:00
Ray Kraesig
dbdb9db19d Bug 1785162 - Disable all-process stalling in Nightly r=gsvelto
A recent patch for bug 965392 (now reverted) may have been causing
additional memory use and OOM-crashes. To confirm this, temporarily(?)
revert all-process stalling on Nightly to the main-process-only version
active between 2022-07-28 and 2022-08-25, to collect a few days' worth of
telemetry.

On Beta and later, there are no functional changes.

Differential Revision: https://phabricator.services.mozilla.com/D157124
2022-09-13 18:34:43 +00:00
Ray Kraesig
709d56796c Bug 1785162 - Always stall on OOM on Windows in Nightly r=gsvelto
It's suspected that this may induce performance regressions. Do it
anyway, just to find out how bad it is. (But only on Nightly, for now.)

For simplicity's sake, this does not include any additional telemetry;
the decision of whether and what any additional telemetry is needed will
be deferred until we have some feedback from what we've already got.

Differential Revision: https://phabricator.services.mozilla.com/D155301
2022-08-25 19:35:32 +00:00
Ray Kraesig
613c6089f5 Bug 1716727 - patchset #3 [1/1] - Cleanup: remove intermediate function r=glandium
Since mozglue is now aware of Gecko process-types, remove the
configurator for this bug's associated experiment. Instead, use
`GeckoProcessType` directly in mozjemalloc.

This requires a couple of adjustments for non-Firefox uses of
mozjemalloc:

- Since `logalloc-replay` uses mozjemalloc in an odd way, tweak its
  `moz.build` to also include `mozglue/misc/ProcessType.cpp`.
- Since `GeckoProcessType` is not available in standalone SpiderMonkey
  builds, make the mostly-arbitrarily choice to always stall there.

No functional changes.

Differential Revision: https://phabricator.services.mozilla.com/D155300
2022-08-25 14:44:47 +00:00
Gabriele Svelto
44cbaf4248 Bug 1784018 - Remove deprecated OSSpinLocks r=glandium
On macOS versions prior to 10.15 os_unfair_locks cannot spin in kernel-space
which degrades performance significantly. To obviate for this we spin in
user-space like OSSpinLock does, for the same number of times and invoking
x86-specific pause instructions in-between the locking attempts to avoid
starving a thread that might be running on the same physical core.

Differential Revision: https://phabricator.services.mozilla.com/D154205
2022-08-24 09:18:57 +00:00
Ray Kraesig
52e0d53fb0 Bug 1716727 - patchset #2 [2/2] Deexperimentalize stalling code r=glandium
Although further experiments and refinements are called for, the effect
of this patch has been overall significantly positive. Let it ride the
trains as-is for Fx 105, rather than blocking on those experiments.

(This patch conflicts with D153784, which will need to be rebased and
moved to a new Bugzilla bug.)

Depends on D153783

Differential Revision: https://phabricator.services.mozilla.com/D154839
2022-08-18 20:32:26 +00:00
Ray Kraesig
b70021268a Bug 1716727 - patchset #2 [1/2] - Assorted minor cleanup r=glandium,bobowen
Since the plausible timeframe for immediate panicked reversion of the
previous patchset has passed without incident, unpin and clean up the
remaining uses of `sChildProcessType` from nsEmbedFunctions.cpp.

Additionally, get rid of the rump declaration of `..._set_always_stall`
on non-Windows builds.

No functional changes.

(Reference: bug 1682520, D152198.)

Differential Revision: https://phabricator.services.mozilla.com/D153783
2022-08-18 20:32:26 +00:00
Fabrice Desré
c50cb528fc Bug 1761040 - Prefix thread safety macros with MOZ_ r=geckoview-reviewers,media-playback-reviewers,alwu,jesup,m_kato
Differential Revision: https://phabricator.services.mozilla.com/D152575
2022-08-03 16:39:41 +00:00
Andreea Pavel
3ccd75af8d Backed out changeset b9d2965591b9 (bug 1761040) for landing with wrong author CLOSED TREE DONTBUILD 2022-08-03 18:55:00 +03:00
Andreea Pavel
fdb7cb2ecd Bug 1761040 - Prefix thread safety macros with MOZ_ r=geckoview-reviewers,media-playback-reviewers,alwu,jesup,m_kato
Differential Revision: https://phabricator.services.mozilla.com/D152575
2022-08-03 15:27:43 +00:00
Andreea Pavel
89d63c91e6 Backed out changeset a907159a482f (bug 1761040) for causing build bustages on a CLOSED TREE 2022-08-02 04:59:08 +03:00
Fabrice Desré
0f4ac7ad97 Bug 1761040 - Prefix thread safety macros with MOZ_ r=geckoview-reviewers,media-playback-reviewers,alwu,jesup,m_kato
Differential Revision: https://phabricator.services.mozilla.com/D152575
2022-08-02 00:49:41 +00:00
Ray Kraesig
5f25d69d33 Bug 1782027 - Fix bustage in simulated early-beta Windows builds r=gcp
... which was entirely due to a trivial error on my part.

Differential Revision: https://phabricator.services.mozilla.com/D153109
2022-07-28 19:11:20 +00:00
Ray Kraesig
a63e2430bf Bug 1716727 - [3/3] make stalling behavior conditional on process type r=glandium
For now, make Set_XREProcessType set a flag in mozjemalloc to avoid
stalling repeatedly in auxiliary processes.

Differential Revision: https://phabricator.services.mozilla.com/D151332
2022-07-27 19:54:36 +00:00
Ray Kraesig
44a45dc8a5 Bug 1716727 - [2/3] OOM handling: stall and retry r=glandium,gsvelto
Implement a drop-in-replacement wrapper for `VirtualAlloc` which, rather
than returning immediately on failure, instead `::Sleep()`s and retries.

This will cause performance regressions in some (presumed-uncommon)
circumstances; this wrapper is therefore only enabled in Nightly, to
collect data on its efficacy and on the severity of those regressions.

Differential Revision: https://phabricator.services.mozilla.com/D150619
2022-07-27 19:54:36 +00:00
Ray Kraesig
ca8b982e11 Bug 1716727 - [1/3] Drive-by cleanup: clang-tidy nitpicks r=glandium
Some trivially-resolvable nitpicks pointed out by clang-tidy.

Differential Revision: https://phabricator.services.mozilla.com/D150618
2022-07-27 19:54:35 +00:00
Butkovits Atila
1402637e8b Backed out 3 changesets (bug 1716727) for causing build bustages. CLOSED TREE
Backed out changeset 11fb3262cccb (bug 1716727)
Backed out changeset 895ad6545236 (bug 1716727)
Backed out changeset c0fd4f429cd8 (bug 1716727)
2022-07-27 04:01:16 +03:00
Ray Kraesig
4ec2a2d0b4 Bug 1716727 - [3/3] make stalling behavior conditional on process type r=glandium
For now, make Set_XREProcessType set a flag in mozjemalloc to avoid
stalling repeatedly in auxiliary processes.

Differential Revision: https://phabricator.services.mozilla.com/D151332
2022-07-27 00:30:04 +00:00
Ray Kraesig
55f93cc62f Bug 1716727 - [2/3] OOM handling: stall and retry r=glandium,gsvelto
Implement a drop-in-replacement wrapper for `VirtualAlloc` which, rather
than returning immediately on failure, instead `::Sleep()`s and retries.

This will cause performance regressions in some (presumed-uncommon)
circumstances; this wrapper is therefore only enabled in Nightly, to
collect data on its efficacy and on the severity of those regressions.

Differential Revision: https://phabricator.services.mozilla.com/D150619
2022-07-27 00:30:04 +00:00
Ray Kraesig
1b26c65a01 Bug 1716727 - [1/3] Drive-by cleanup: clang-tidy nitpicks r=glandium
Some trivially-resolvable nitpicks pointed out by clang-tidy.

Differential Revision: https://phabricator.services.mozilla.com/D150618
2022-07-27 00:30:03 +00:00
Paul Bone
7a49f8892c Bug 1742828 - Enable more aggressive allocation packing on MacOS r=glandium
Differential Revision: https://phabricator.services.mozilla.com/D139800
2022-07-26 23:54:17 +00:00
Ray Kraesig
1f4a9dd483 Bug 1207753 - add thread-safety annotations to memory/build's Mutex types r=glandium,jesup
Add the customary bevy of thread-safety annotations to the Mutex types
in memory/build.

Almost all existing instances of these Mutexes are already marked with
MOZ_UNANNOTATED, but this turns out not to matter: MOZ_UNANNOTATED has
no equivalent under clang's `-Wthread-safety` attributes. There are no
variables declared to be guarded by these mutexes, but they must still
be unlocked and relocked in principled ways -- or, alternatively, only
have unverifiable actions taken in appropriately-annotated code.

The primary offenders here are the `pthread_atfork` hooks; these can't
all be annotated precisely (and couldn't be checked even if they were),
so we simply mark them all as unanalyzable. The only other warning is
from a highly unusual (but already well-commented) bit in PHC.cpp where
we manually release an autolocked mutex before crashing.

Differential Revision: https://phabricator.services.mozilla.com/D150519
2022-07-08 21:24:03 +00:00