Mike Hommey [:glandium] Assignee
	Description • 7 years ago

      No description provided.

	Nicholas Nethercote [inactive]
	Comment 22 • 7 years ago
mozreview-review

Comment on attachment 8868048 [details]
Bug 1365191 - Remove #if 0 sections in mozjemalloc.

https://reviewboard.mozilla.org/r/139644/#review143246

	Nicholas Nethercote [inactive]
	Comment 23 • 7 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.

	Nicholas Nethercote [inactive]
	Comment 24 • 7 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.

	Nicholas Nethercote [inactive]
	Comment 25 • 7 years ago
mozreview-review

Comment on attachment 8868051 [details]
Bug 1365191 - Remove MALLOC_UTRACE from mozjemalloc.

https://reviewboard.mozilla.org/r/139650/#review143252

	Nicholas Nethercote [inactive]
	Comment 26 • 7 years ago
mozreview-review

Comment on attachment 8868052 [details]
Bug 1365191 - Remove MALLOC_STATS from mozjemalloc.

https://reviewboard.mozilla.org/r/139652/#review143254

	Mike Hommey [:glandium] Assignee
	Comment 27 • 7 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.

	Nicholas Nethercote [inactive]
	Comment 28 • 7 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?

	Nicholas Nethercote [inactive]
	Comment 29 • 7 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

	Mike Hommey [:glandium] Assignee
	Comment 30 • 7 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...

	Nicholas Nethercote [inactive]
	Comment 31 • 7 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.

	Pulsebot
	Comment 32 • 7 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

	Carsten Book [:Tomcat]
	Comment 33 • 7 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