I dont know mozilla's stance wrt jemalloc, i never really looked into it since we're pretty happy with our own malloc, and i also find it somewhat stupid to not rely on the operating system malloc... but that said, build is broken by default for me since the 22 when bug 1134039 landed: 1:29.84 js/src> configure: error: --enable-jemalloc not supported on i386-unknown-openbsd5.7 So i need to know if i should sprinkle --disable-jemalloc all around in my mozconfig, or if the commit can be amended for the unsupported platforms, or if there's a better way...

Here's what i had in mind, and a similar thing for toplevel configure.in; In the OpenBSD community, nobody trusts jemalloc as it is "finely tuned to turn off ASLR" (not my words) and the (supposed) performance improvements are not worth the risks/regression it brings. Personally, i think that not relying on the OS's malloc is wrong, so can we please disable it for OpenBSD at least ?

Comment on attachment 8587959 [details] [diff] [review] disable-jemalloc Review of attachment 8587959 [details] [diff] [review]: ----------------------------------------------------------------- You should try something like this instead: diff --git a/js/src/configure.in b/js/src/configure.in index 978d370..61be6bb 100644 --- a/js/src/configure.in +++ b/js/src/configure.in @@ -3020,29 +3020,26 @@ if test "$MOZ_MEMORY"; then fi dnl The generic feature tests that determine how to compute ncpus are long and dnl complicated. Therefore, simply define special cpp variables for the dnl platforms we have special knowledge of. case "${target}" in *-darwin*) AC_DEFINE(MOZ_MEMORY_DARWIN) ;; - *-*freebsd*) + *-*bsd*) AC_DEFINE(MOZ_MEMORY_BSD) ;; *-android*|*-linuxandroid*) AC_DEFINE(MOZ_MEMORY_LINUX) AC_DEFINE(MOZ_MEMORY_ANDROID) ;; *-*linux*) AC_DEFINE(MOZ_MEMORY_LINUX) ;; - *-netbsd*) - AC_DEFINE(MOZ_MEMORY_BSD) - ;; *-solaris*) AC_DEFINE(MOZ_MEMORY_SOLARIS) ;; *-mingw*) AC_DEFINE(MOZ_MEMORY_WINDOWS) ;; *) AC_MSG_ERROR([--enable-jemalloc not supported on ${target}])

(In reply to Landry Breuil (:gaston) from comment #1) >@@ -2999,16 +2999,21 @@ dnl ==================================== > dnl = Enable jemalloc > dnl ======================================================== > MOZ_MEMORY=1 Yuck! mozjemalloc has bitrotten support for Tier3. I need to disable it on FreeBSD as well until bug 762451. > MOZ_ARG_DISABLE_BOOL(jemalloc, > [ --disable-jemalloc Don't replace memory allocator with jemalloc], > MOZ_MEMORY=) > > case "${OS_TARGET}" in >+OpenBSD) >+ dnl we dont want to use jemalloc on OpenBSD >+ MOZ_MEMORY= Opting out should be before MOZ_ARG_* handling to allow experimental builds with --enable-jemalloc. Example use case is debugging regressions when using system malloc (being non-default) in Firefox or in malloc itself (either one). (In reply to Mike Hommey [:glandium] from comment #2) > - *-*freebsd*) > + *-*bsd*) > AC_DEFINE(MOZ_MEMORY_BSD) > ;; DragonFly doesn't have BSD suffix in |uname -s|. Not sure if it's affected by bug 788955 comment 25. https://github.com/DragonFlyBSD/DeltaPorts/blob/master/ports/www/firefox/dragonfly/patch-configure.in

configure:MOZ_JEMALLOC="" vs. js/src/configure.in:MOZ_JEMALLOC=1 leads to an interesting error: Traceback (most recent call last): File "./config.status", line 989, in <module> config_status(**args) File "python/mozbuild/mozbuild/config_status.py", line 149, in config_status summary = the_backend.consume(definitions) File "python/mozbuild/mozbuild/backend/base.py", line 180, in consume for obj in objs: File "python/mozbuild/mozbuild/frontend/emitter.py", line 168, in emit objs = list(self._emit_libs_derived(contexts)) File "python/mozbuild/mozbuild/frontend/emitter.py", line 204, in _emit_libs_derived self._link_libraries(context, obj, variable) File "python/mozbuild/mozbuild/frontend/emitter.py", line 317, in _link_libraries context) mozbuild.frontend.reader.SandboxValidationError: ============================== ERROR PROCESSING MOZBUILD FILE ============================== The error occurred while processing the following file or one of the files it includes: js/src/shell/moz.build The error occurred when validating the result of the execution. The reported error is: USE_LIBS contains "memory", which does not match any LIBRARY_NAME in the tree. http://buildbot.rhaalovely.net/builders/mozilla-aurora-netbsd-amd64/builds/72/steps/configure/logs/stdio http://buildbot.rhaalovely.net/builders/mozilla-aurora-freebsd-amd64/builds/585/steps/configure/logs/stdio

(In reply to Jan Beich from comment #4) > configure:MOZ_JEMALLOC="" vs. js/src/configure.in:MOZ_JEMALLOC=1 leads... Sorry typos. I meant configure.in:MOZ_MEMORY="" vs. js/src/configure.in:MOZ_MEMORY=1, i.e. memory/mozjemalloc/ lives outside of js/src/ and may not be built without JS_STANDALONE.

Maybe sync MOZ_ARG_* and MOZ_MEMORY=1 between configure.in and js/src/configure.in ? https://treeherder.mozilla.org/#/jobs?repo=try&revision=44e7c305ba90

(In reply to Mike Hommey [:glandium] from comment #2) > Comment on attachment 8587959 [details] [diff] [review] > disable-jemalloc > > Review of attachment 8587959 [details] [diff] [review]: > ----------------------------------------------------------------- > > You should try something like this instead: As i said, i dont really care about jemalloc. The way it works hides possible possible use-after-free and overflows, so we should always be able to override that and use our own system malloc, since it *will* uncover more bugs in firefox itself, which would be hidden by jemalloc otherwise. So yeah, we might "fix" jemalloc-by-default using such a diff, but the background issue will still be there. People might want to use their system malloc for various reasons (debugging, as jan said), and you shouldnt remove that option from them.

Comment on attachment 8588264 [details] [diff] [review] consistent defaults, v0 I dont really understand all the intricacies between MOZ_JEMALLOC3 and MOZ_MEMORY but with that diff i can run configure, and have MOZ_MEMORY properly defaulting to unset on OpenBSD.

(In reply to Landry Breuil (:gaston) from comment #7) > As i said, i dont really care about jemalloc. The way it works hides > possible possible use-after-free No it doesn't. It even poisons freed memory. > and overflows No allocator can prevent overflows.

(In reply to Mike Hommey [:glandium] from comment #9) > (In reply to Landry Breuil (:gaston) from comment #7) > > As i said, i dont really care about jemalloc. The way it works hides > > possible possible use-after-free > > No it doesn't. It even poisons freed memory. And what do you think our own malloc do ? Oh well. I dont have the arguments to debate this, and bikesheds are not my thing. Other OpenBSD developers who understand allocators *might* chime in, but i know it's a lost cause. Now even if i try to 'fix' jemalloc the way you propose in comment 2, the build is broken as shown by jan in comment 4.

Comment on attachment 8588264 [details] [diff] [review] consistent defaults, v0 Review of attachment 8588264 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +7145,5 @@ > + MOZ_MEMORY=1 > + fi > + ;; > + esac > +fi This should just be MOZ_MEMORY=1 everywhere but osx 32 bits. OpenBSD can use --disable-jemalloc in their ports if they really want to disable it. And the native jemalloc code path should be changed, I think, not to be triggered on MOZ_MEMORY being empty. Note that mingw builds currently don't enable jemalloc, but I don't know why. Jacek, do you know?

Comment on attachment 8588264 [details] [diff] [review] consistent defaults, v0 Patch got bitrotted and doesnt apply anymore..... Can we do something about this issue please ? Tree not building on all BSDs is not nice.

(In reply to Mike Hommey [:glandium] from comment #11) > Note that mingw builds currently don't enable jemalloc, but I don't know > why. Jacek, do you know? I don't know, probably some build problems. I will try to experiment with enabling it.

Can we move forward please and actually commit anything that fixes the breakage ? It's been a week and configure not working for all BSDs might hide actual build issues piling on top from showing up in buildbot...

(In reply to Landry Breuil (:gaston) from comment #12) > Patch got bitrotted and doesnt apply anymore..... Hmm, maybe my local commits affected |diff -U8| context. Trying again and fixing whitespace while here. If it doesn't help apply the change from the Try build in comment 6. (In reply to Mike Hommey [:glandium] from comment #11) Can you stop cancelling review without explaining why? This is not the same as r- or r+ on condition and can be interpreted as "not willing to be a reviewer for the patch". https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Addressing_review_comments > ::: configure.in > @@ +7145,5 @@ > > + MOZ_MEMORY=1 > > + fi > > + ;; > > + esac > > +fi > > This should just be MOZ_MEMORY=1 everywhere but osx 32 bits. Do you expect platform-specific codepaths in mozjemalloc to just magically work after years being unused and untested since the fork? Maybe Landry can test --enable-jemalloc after applying comment 2 and the attached patch. My experience is in bug 1153683. > OpenBSD can use --disable-jemalloc in their ports if they really > want to disable it. Same as bug 952828 comment 14. I'd force the quirk/tuning downstream as well if there was no active maintainer here. ;) > And the native jemalloc code path should be changed, I think, not to > be triggered on MOZ_MEMORY being empty. This belongs in a different bug. Note, native jemalloc if in libc cannot be disabled sans memory stats/tuning under -DMOZ_MEMORY.

Comment on attachment 8591419 [details] [diff] [review] consistent defaults, v0 Review of attachment 8591419 [details] [diff] [review]: ----------------------------------------------------------------- Let's see how bug 1155438 changes things first.

(In reply to Mike Hommey [:glandium] from comment #16) > Let's see how bug 1155438 changes things first. That bug is about related issue. It only addresses the error in comment 4. Other issues such as comment 0 (and comment 2) or exposed bug 1153683 still remain. Not to mention disabling jemalloc only for Tier3 and only for non-standalone is confusing, as if Tier3 runtime receives enough care for such distinction. About consistency, I wonder if bug 702250 doesn't affect standalone JS.

Rebased.

Comment on attachment 8598107 [details] [diff] [review] consistent defaults, v0 Fwiw, i like this.

Landry, are you going to fix this? If not Assignee is misleading.

I dont remember what made the issue/configure error go away, but i'm far from looking into this again...

Just to double-check: are you saying the issue doesn't reproduce? If so, maybe we could close this bug.

It still affects at least FreeBSD. Standalone SpiderMonkey crashes due to bug 1153683 caused by implicit --enable-jemalloc.

The 'original' issue (ie jemalloc being forced on everywhere) see comment #0, configure: error: --enable-jemalloc not supported on i386-unknown-openbsd5.7) has been probably fixed since then, ie jemalloc is not be enabled by default on OpenBSD now. If i pass --enable-jemalloc manually it fails: 0:35.68 ERROR: --enable-jemalloc is not supported on OpenBSD

Have you tried standalone build? $ cd js/src $ autoconf-2.13 $ ./configure $ gmake $ js/src/shell/js According build/moz.configure/memory.configure it should fail for OpenBSD as if --enable-jemalloc was explicitly passed. @depends('--enable-jemalloc', target, build_project, c_compiler) def jemalloc(value, target, build_project, c_compiler): if value.origin != 'default': return bool(value) or None if build_project == 'js': return True

[14:33] openbsd-amd64:~/src/mozilla-central/js/src/ $CC=clang CXX=clang++ ./configure Reexecuting in the virtualenv checking for a shell... /bin/sh checking for host system type... x86_64-unknown-openbsd6.0 checking for target system type... x86_64-unknown-openbsd6.0 checking for the Android toolchain directory... not found checking whether cross compiling... no checking for pkg_config... /usr/bin/pkg-config checking for pkg-config version... 0.27.1 checking for yasm... /usr/local/bin/yasm checking yasm version... 1.3.0 checking for android platform directory... no checking for the target C compiler... /usr/local/bin/clang checking whether the target C compiler can be used... yes checking the target C compiler version... 3.9.0 checking the target C compiler works... yes checking for the target C++ compiler... /usr/local/bin/clang++ checking whether the target C++ compiler can be used... yes checking the target C++ compiler version... 3.9.0 checking the target C++ compiler works... yes checking for the host C compiler... /usr/local/bin/clang checking whether the host C compiler can be used... yes checking the host C compiler version... 3.9.0 checking the host C compiler works... yes checking for the host C++ compiler... /usr/local/bin/clang++ checking whether the host C++ compiler can be used... yes checking the host C++ compiler version... 3.9.0 checking the host C++ compiler works... yes checking for 64-bit OS... yes ERROR: --enable-jemalloc is not supported on OpenBSD [14:34] openbsd-amd64:~/src/mozilla-central/js/src/

Probably not an issue after https://hg.mozilla.org/mozilla-central/rev/e8ac1244375a