1339075 - stylo: forces mozjemalloc which is unusable on Tier3

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Jan Beich]

mozjemalloc is broken on Tier3 platforms (e.g. bug 1153683) while bug 762449 keeps getting pushed back for years.

$ pkg install llvm39 python27
$ echo "export LLVM_CONFIG=$(which llvm-config39)" >>.mozconfig
$ echo "ac_add_options --enable-stylo" >>.mozconfig
$ ./mach bootstrap
$ ./mach build

$ ./mach run
 0:00.20 obj-x86_64-unknown-freebsd12.0/dist/bin/firefox -no-remote -profile obj-x86_64-unknown-freebsd12.0/tmp/scratch_user
Exit 245

$ lldb -- obj-x86_64-unknown-freebsd12.0/dist/bin/firefox -no-remote -profile obj-x86_64-unknown-freebsd12.0/tmp/scratch_user
(lldb) r
Process 471 launching
Process 471 launched: 'obj-x86_64-unknown-freebsd12.0/dist/bin/firefox' (x86_64)
Process 471 stopped
* thread #1, stop reason = signal SIGSEGV: invalid address (fault address: 0x7fffdfffef18)
    frame #0: firefox`malloc_init [inlined] malloc_mutex_lock(mutex=0x00000000004299b8) at jemalloc.c:1642
   1639 #elif defined(MOZ_MEMORY_DARWIN)
   1640         OSSpinLockLock(&mutex->lock);
   1641 #elif defined(MOZ_MEMORY)
-> 1642         pthread_mutex_lock(mutex);
   1643 #else
   1644         if (isthreaded)
   1645                 _SPINLOCK(&mutex->lock);

$ echo "ac_add_options --enable-jemalloc=4" >>.mozconfig
$ ./mach build
[...]
 0:03.08 mozbuild.configure.options.InvalidOptionError: '--enable-jemalloc=moz' implied by '--enable-dmd or --enable-stylo' conflicts with '--enable-jemalloc=4' from the mozconfig

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Note that we have a hack in place (see bug 1291356) to enable multiple jemalloc arenas for stylo due to the parallel allocations that were killing our performance. I believe the two options there before shipping are either keeping that hack or updating to jemalloc 4.

Comment 2

[Bobby Holley (:bholley)]

Per bug 1291355, we may not need multiple arenas anymore, but we don't have enough data to say anything conclusive. Hopefully we'll know more in a few weeks.

Comment 3

[Jan Beich]

Landry, does OpenBSD use jemalloc or system malloc with Rust apps (check with a tracing tool)? If the latter can you try building Stylo against it then run on a simple page? A quick search[1] suggests Rust linker doesn't enable jemalloc on NetBSD, OpenBSD by default.

[1] https://github.com/rust-lang/rust/search?q=maybe_jemalloc

Comment 4

[Bobby Holley (:bholley)]

To be clear, we don't expect the existing "force mozjemalloc" behavior to ship with stylo, it's just there to avoid a moving performance target if the default allocator is changed. Tier-3 platforms shouldn't worry about this - if the build is failing we should just disabling the mozjemalloc-forcing behavior on tier-3 builds.

Comment 5

[Landry Breuil (:gaston)]

I have absolutely no idea about jemalloc and rust :) And i dont think ktrace will tell me..

Comment 6

[Sebastien Marie [:semarie]]

Under OpenBSD, Rust apps use system malloc by default.

jemalloc supports a bit exotic with OpenBSD: last time I checked depending if jemalloc is compiled with gcc 4.2 (from base) or gcc 4.9 (from ports), the behaviour was different. It was related to thread-local-storage detection if I recall correctly.

Comment 7

[Jan Beich]

Attachment: Bug 1339075 - stylo: don't force mozjemalloc on Tier3 platforms.

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

Comment 8

[Nathan Froyd [:froydnj]]

Comment on attachment 8839834 [details]
Bug 1339075 - stylo: don't force mozjemalloc on Tier3 platforms.

https://reviewboard.mozilla.org/r/114412/#review115978

It might be worth a comment about why we're picking those platforms (e.g. "Enable jemalloc for stylo only on Tier1 platforms"), but I'm not sure it's worth insisting on.

Comment 9

[Jan Beich]

Comment on attachment 8839834 [details]
Bug 1339075 - stylo: don't force mozjemalloc on Tier3 platforms.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/114412/diff/1-2/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/27dade5e0c83
stylo: don't force mozjemalloc on Tier3 platforms. r=froydnj

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/27dade5e0c83