Closed
Bug 1365191
Opened 4 years ago
Closed 4 years ago
Some mozjemalloc code cleanup
Categories
(Core :: Memory Allocator, enhancement)
Core
Memory Allocator
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: glandium, Assigned: glandium)
References
Details
Attachments
(7 files)
59 bytes,
text/x-review-board-request
|
n.nethercote
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
n.nethercote
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
n.nethercote
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
n.nethercote
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
n.nethercote
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
n.nethercote
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
n.nethercote
:
review+
|
Details |
No description provided.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
![]() |
||
Comment 22•4 years ago
|
||
mozreview-review |
Comment on attachment 8868048 [details] Bug 1365191 - Remove #if 0 sections in mozjemalloc. https://reviewboard.mozilla.org/r/139644/#review143246
Attachment #8868048 -
Flags: review?(n.nethercote) → review+
![]() |
||
Comment 23•4 years ago
|
||
mozreview-review |
Comment on attachment 8868049 [details] Bug 1365191 - Remove !MOZ_MEMORY sections in mozjemalloc. https://reviewboard.mozilla.org/r/139646/#review143248 ::: memory/mozjemalloc/jemalloc.c (Diff revision 3) > #include <limits.h> > #ifndef SIZE_T_MAX > # define SIZE_T_MAX SIZE_MAX > #endif > #include <pthread.h> > -#ifdef MOZ_MEMORY_DARWIN This change seems out of place in this patch.
Attachment #8868049 -
Flags: review?(n.nethercote) → review+
![]() |
||
Comment 24•4 years ago
|
||
mozreview-review |
Comment on attachment 8868050 [details] Bug 1365191 - Remove MALLOC_VALIDATE in mozjemalloc. https://reviewboard.mozilla.org/r/139648/#review143250 I feel slightly bad that we're losing documentation about which parts of the code are just for validation, but if we always validate that feeling is probably not helpful.
Attachment #8868050 -
Flags: review?(n.nethercote) → review+
![]() |
||
Comment 25•4 years ago
|
||
mozreview-review |
Comment on attachment 8868051 [details] Bug 1365191 - Remove MALLOC_UTRACE from mozjemalloc. https://reviewboard.mozilla.org/r/139650/#review143252
Attachment #8868051 -
Flags: review?(n.nethercote) → review+
![]() |
||
Comment 26•4 years ago
|
||
mozreview-review |
Comment on attachment 8868052 [details] Bug 1365191 - Remove MALLOC_STATS from mozjemalloc. https://reviewboard.mozilla.org/r/139652/#review143254
Attachment #8868052 -
Flags: review?(n.nethercote) → review+
Assignee | ||
Comment 27•4 years ago
|
||
mozreview-review-reply |
Comment on attachment 8868049 [details] Bug 1365191 - Remove !MOZ_MEMORY sections in mozjemalloc. https://reviewboard.mozilla.org/r/139646/#review143248 > This change seems out of place in this patch. This changes comes from the fact that the last uses of those macros are removed in this patch.
![]() |
||
Comment 28•4 years ago
|
||
mozreview-review |
Comment on attachment 8868053 [details] Bug 1365191 - Remove MALLOC_FILL from mozjemalloc. https://reviewboard.mozilla.org/r/139654/#review143258 opt_zero is always false and opt_poison is always true. Is it worth getting rid of those constants?
Attachment #8868053 -
Flags: review?(n.nethercote) → review+
![]() |
||
Comment 29•4 years ago
|
||
mozreview-review |
Comment on attachment 8868054 [details] Bug 1365191 - Remove dead code hidden behind the never set NEEDS_PTHREAD_MMAP_UNALIGNED_TSD. https://reviewboard.mozilla.org/r/139656/#review143260
Attachment #8868054 -
Flags: review?(n.nethercote) → review+
Assignee | ||
Comment 30•4 years ago
|
||
mozreview-review-reply |
Comment on attachment 8868053 [details] Bug 1365191 - Remove MALLOC_FILL from mozjemalloc. https://reviewboard.mozilla.org/r/139654/#review143258 They can both be overridden at runtime with MALLOC_OPTIONS in debug builds. I'm not sure it's something worth keeping or not. I don't think there's much value in disabling poison, but enabling zero at runtime may or may not be useful. Arguably, we could have a replace-malloc lib to provide that...
![]() |
||
Comment 31•4 years ago
|
||
> I don't think there's much value in disabling poison, but enabling zero at runtime may or may not be useful.
I agree that the case for removing opt_poison is stronger than the case for removing opt_zero. I can't imagine ever wanting to disable poisoning.
Comment 32•4 years ago
|
||
Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/c2ac9d5ebe2f Remove #if 0 sections in mozjemalloc. r=njn https://hg.mozilla.org/integration/autoland/rev/03aeae77fd57 Remove !MOZ_MEMORY sections in mozjemalloc. r=njn https://hg.mozilla.org/integration/autoland/rev/a33773e54f5d Remove MALLOC_VALIDATE in mozjemalloc. r=njn https://hg.mozilla.org/integration/autoland/rev/0f4289fed78b Remove MALLOC_UTRACE from mozjemalloc. r=njn https://hg.mozilla.org/integration/autoland/rev/e246baa0d780 Remove MALLOC_STATS from mozjemalloc. r=njn https://hg.mozilla.org/integration/autoland/rev/09b32bedead3 Remove MALLOC_FILL from mozjemalloc. r=njn https://hg.mozilla.org/integration/autoland/rev/a0c7189d60e3 Remove dead code hidden behind the never set NEEDS_PTHREAD_MMAP_UNALIGNED_TSD. r=njn
Comment 33•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c2ac9d5ebe2f https://hg.mozilla.org/mozilla-central/rev/03aeae77fd57 https://hg.mozilla.org/mozilla-central/rev/a33773e54f5d https://hg.mozilla.org/mozilla-central/rev/0f4289fed78b https://hg.mozilla.org/mozilla-central/rev/e246baa0d780 https://hg.mozilla.org/mozilla-central/rev/09b32bedead3 https://hg.mozilla.org/mozilla-central/rev/a0c7189d60e3
Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•