Unfortunately, this depends on being able to distinguish between msvc and gcc on Windows, which depends on at least CC being moved to moz.configure.

This was added in bug 1159371 but doesn't seem necessary anymore. Review commit: https://reviewboard.mozilla.org/r/46373/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46373/

At the same time, allow to enable jemalloc 4 with --enable-jemalloc=4. MOZ_JEMALLOC4 will be deprecated later. This also changes the semantics for freebsd, where the system jemalloc is used, relying on MOZ_MEMORY being unset (default on freebsd) and MOZ_JEMALLOC4 to be set. In this new setup, MOZ_JEMALLOC4 implies --enable-jemalloc=4, which still works because of the corresponding changes to old-configure. Review commit: https://reviewboard.mozilla.org/r/46375/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46375/

Review commit: https://reviewboard.mozilla.org/r/46377/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46377/

Review commit: https://reviewboard.mozilla.org/r/46379/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46379/

Comment on attachment 8741309 [details] MozReview Request: Bug 1257448 - Don't disable MOZ_MEMORY when building fennec with --disable-compile-environment. r?nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46373/diff/1-2/

Comment on attachment 8741310 [details] MozReview Request: Bug 1257448 - Move --enable-jemalloc and MOZ_JEMALLOC4 to moz.configure. r?nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46375/diff/1-2/

Comment on attachment 8741311 [details] MozReview Request: Bug 1257448 - Move MOZ_MEMORY_* defines to moz.configure. r?nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46377/diff/1-2/

Comment on attachment 8741312 [details] MozReview Request: Bug 1257448 - Move --enable-replace-malloc to moz.configure. r?nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46379/diff/1-2/

Comment on attachment 8741312 [details] MozReview Request: Bug 1257448 - Move --enable-replace-malloc to moz.configure. r?nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46379/diff/2-3/

Comment on attachment 8741309 [details] MozReview Request: Bug 1257448 - Don't disable MOZ_MEMORY when building fennec with --disable-compile-environment. r?nalexander https://reviewboard.mozilla.org/r/46373/#review43051 If it works for you, it works for me.

Comment on attachment 8741310 [details] MozReview Request: Bug 1257448 - Move --enable-jemalloc and MOZ_JEMALLOC4 to moz.configure. r?nalexander https://reviewboard.mozilla.org/r/46375/#review43053 ::: build/moz.configure/memory.configure:15 (Diff revision 2) > + > + > +option('--enable-jemalloc', nargs='?', choices=('4',), env='MOZ_MEMORY', > + help='Replace memory allocator with jemalloc') > + > +@depends('--enable-jemalloc', target, build_project, c_compiler) Further to my comment about `no-toolchain.configure`, why doesn't this depend on *having* a toolchain? It seems like this breaks the toolchain abstraction. ::: build/moz.configure/no-toolchain.configure:17 (Diff revision 2) > + flags=(), > + type='', > + version='', > + ) > + > +c_compiler = no_toolchain_compiler This appears to build on some unlanded code, so I can't try to interpret it. (I see no hits for, say, https://dxr.mozilla.org/mozilla-central/search?tree=mozilla-central&q=host_cxx_compiler&redirect=true) In general, I can't understand why exposing these symbols when we don't actually have a toolchain is a good idea.

Comment on attachment 8741311 [details] MozReview Request: Bug 1257448 - Move MOZ_MEMORY_* defines to moz.configure. r?nalexander https://reviewboard.mozilla.org/r/46377/#review43059 ::: build/moz.configure/memory.configure:76 (Diff revision 2) > + die('--enable-jemalloc is not supported on %s', target.kernel) > + > +set_define(jemalloc_os_define, '1') > + > +@depends(jemalloc, jemalloc4, target) > +def jemalloc_os_define_android(jemalloc, jemalloc4, target): I assume this is separate 'cuz you want to define both `MOZ_MEMORY_LINUX` and `MOZ_MEMORY_ANDROID` on Android. Comment if I'm wrong.

Comment on attachment 8741312 [details] MozReview Request: Bug 1257448 - Move --enable-replace-malloc to moz.configure. r?nalexander https://reviewboard.mozilla.org/r/46379/#review43061 ::: toolkit/moz.configure:69 (Diff revision 3) > set_config('MOZ_DMD', dmd) > set_define('MOZ_DMD', dmd) > add_old_configure_assignment('MOZ_DMD', dmd) > imply_option('--enable-profiling', dmd) > imply_option('--enable-jemalloc', dmd) > +imply_option('--enable-replace-malloc', dmd) So much better!

(In reply to Nick Alexander :nalexander from comment #12) > ::: build/moz.configure/no-toolchain.configure:17 > (Diff revision 2) > > + flags=(), > > + type='', > > + version='', > > + ) > > + > > +c_compiler = no_toolchain_compiler > > This appears to build on some unlanded code, so I can't try to interpret it. > (I see no hits for, say, > https://dxr.mozilla.org/mozilla-central/search?tree=mozilla- > central&q=host_cxx_compiler&redirect=true) That's because dxr is lagging. It landed yesterday on m-i. https://hg.mozilla.org/integration/mozilla-inbound/file/tip/build/moz.configure/toolchain.configure#l419 > In general, I can't understand why exposing these symbols when we don't > actually have a toolchain is a good idea. So, this is where things are more complicated than "this enables something that is compiled". That might not be true for jemalloc, in fact, but we have plenty of things that will require something like no-toolchain, because disable-compile-environment builds still need the defines or substs to enable the js part of the code, or artifact builds need them for the package manifest. I guess we can wait for the next occasion. I'll just skip-include memory.configure when disable-compile-environment is passed, for now. It looks like it wouldn't break anything.

imply_option has no effect when the resolved value is None, so the same logic can be applied when checking for unknown implied options. This allows to imply options that may not always exist (because they are in a configure file that is optionally included). Ideally, it would be better not to do this, but until we have something better than optionally included configure files for --disable-compile-environment, this is a necessary evil. Review commit: https://reviewboard.mozilla.org/r/46549/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/46549/

Comment on attachment 8741311 [details] MozReview Request: Bug 1257448 - Move MOZ_MEMORY_* defines to moz.configure. r?nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46377/diff/2-3/

Comment on attachment 8741312 [details] MozReview Request: Bug 1257448 - Move --enable-replace-malloc to moz.configure. r?nalexander Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46379/diff/3-4/

Comment on attachment 8741528 [details] MozReview Request: Bug 1257448 - Don't emit an error on unknown implied options when their resolved value is None https://reviewboard.mozilla.org/r/46549/#review43195 This approach is fine by me. ::: moz.configure:64 (Diff revision 1) > - return 'build/moz.configure/no-toolchain.configure' > > include(toolchain_include) > > -include('build/moz.configure/memory.configure') > +@depends('--disable-compile-environment', '--help') > +def memory_include(value, help): I find it odd that this depends on the '--disable-c-e' flag, but tests for `value` (being truthy). Can we make it depend on `compile_environment`, so this is more natural?

https://hg.mozilla.org/integration/mozilla-inbound/rev/334804a9afac https://hg.mozilla.org/integration/mozilla-inbound/rev/b848122c64f9 https://hg.mozilla.org/integration/mozilla-inbound/rev/191926442366 https://hg.mozilla.org/integration/mozilla-inbound/rev/e830d7f62e4d https://hg.mozilla.org/integration/mozilla-inbound/rev/f521d5b4368e

https://hg.mozilla.org/mozilla-central/rev/334804a9afac https://hg.mozilla.org/mozilla-central/rev/b848122c64f9 https://hg.mozilla.org/mozilla-central/rev/191926442366 https://hg.mozilla.org/mozilla-central/rev/e830d7f62e4d https://hg.mozilla.org/mozilla-central/rev/f521d5b4368e