There're some benefits for using libjemalloc.so. 1) User can switch to another malloc libraries if libjemalloc.so doesn't work for him. Use LD_PRELOAD. e.g. To use a malloc libraries with debugging functions, or just switch to libc.so 2) Old version of Solaris Nevada, and Solaris 10 doesn't have posix_memalign in libc.so. That will be a problem with libmozjs.so + static libjemalloc. If we don't link libmozjs.so with static libjemalloc, yelp will fail, lacking posix_memalign. If we link libmozjs.so with static libjemalloc, I think it will cause a memory leak with yelp, because libmozjs.so will use its own posix_memalign and libc's free(). And Firefox will have 2 jemalloc libraries, one in firefox-bin, one in libmozjs.so. If we use shard lib of jemalloc, the problem we have is xulrunner-stub. I think we can live with it. The xulrunner app owner can work out their own solution on Solaris.

Since I'm already touching this file, I add another Solaris specific patch. Use "-z interpose" for libjemalloc.so for more robust. Some plugins may dlopen their libraries with RTLD_GROUP flag, then their libraries will bind to libc.so's malloc/free, therefore it may cause a crash.

I don't think this is a good idea. It means that embedders will not be able to dynamically load gecko the way they can on every other platform. I would prefer to make spidermonkey not use posix_memalign (even when jemalloc is enabled) on Solaris.

It's not only about spidermonkey and posix_memalign. The other reasons about not making jemalloc static are, 1) We don't want to statically link libjemalloc.a to multiple binaries, e.g. xulrunner-bin/firefox-bin and xulrunner stub. 2) To have the ability to override libjemalloc by LD_PRELOAD. embedders is able to dynamically load gecko, it loads libxul and libjs, and libjs loads libjemalloc, everything works great. If the embedder wants to use libjemalloc or other memory allocator, it can statically or dynamically links to its own memory allocatoer library or libjemalloc. The posix_memalign in libjemalloc is a wrapper for memalign. memalign can be bound to the chosen memory allocator library. Making jemalloc static doesn't help the embedder case, if the embedder doesn't build from mozilla-tree. Making jemalloc static would be helpful for xulrunner-stub, but that's not the way we'd like to use on Solaris.

The problem on ELF platforms was this: * the embedder starts up, uses the libc allocator * later, they load libxul, which pulls in libjemalloc.so * this causes everything to switch allocators mid-stream, which obviously causes crashes The solution to this is that embedders *don't* use jemalloc, unless they choose to by statically linking it. You say you don't want to do static linking, but you don't really say why, and the reasons to do static linking are pretty important.

(In reply to comment #5) > The problem on ELF platforms was this: > > * the embedder starts up, uses the libc allocator > * later, they load libxul, which pulls in libjemalloc.so > * this causes everything to switch allocators mid-stream, which obviously > causes crashes > no, it won't switch to jemalloc, at least not on Solaris. > The solution to this is that embedders *don't* use jemalloc, unless they choose > to by statically linking it. since it won't switch and won't crash, it's not a problem, the embedders still have their choices. > You say you don't want to do static linking, but you don't really say why, and > the reasons to do static linking are pretty important. I have already gave the reasons in my last comment. 1) We don't want to statically link libjemalloc.a to multiple binaries, e.g. xulrunner-bin/firefox-bin and xulrunner stub. 2) To have the ability to override libjemalloc by LD_PRELOAD.

I do not think that the reasons you have listed are a compelling reason to diverge from the linking strategy we're using on Linux.

LD_PRELOAD=libumem.so is helpful for plugin developers to detect memory leaks and other problems. It would be a regression that Firefox 3.1 doesn't support it.

Benjamin, When I'm porting jemalloc to Solaris. Every change of linkage of jemalloc would need some changes for Solaris platform. Shared -> into libxul -> shared, and now static. It's really painful procedure. I really want to keep it as a shared library for a end. Because I don't think jemalloc is a one-size-fits-all solution for Solaris. Solaris ships several different malloc libraries for user to choose. User can easily use LD_PRELOAD to override. Can you reconsider your decision? If you insist, may I add an option --with-shared-jemalloc to configure.in? Thanks!

Comment on attachment 343542 [details] [diff] [review] add -z interpose for OS_LDFLAGS This is the only bug that blocks building mozilla-central on OpenSolaris now. I hope we can move forward. FWIW: The suggestion in comment #3 was a reverse of bug 422055 comment #14, and the solution we use right now is bug 422055 comment #19.

I do not want to support jemalloc as a shared library, no. Substituting other allocators via LD_PRELOAD would only work if those allocators exported the exact same set of symbols that jemalloc does, including posix_memalign. I think that is more fragile than the current solution. I think that Solaris should not use jemalloc if the current linkage solution is not acceptable.

It works, since jemalloc's posix_memalign() is a wrapper for memalign().