Last Comment Bug 677501 - Build a mozutils library containing jemalloc and other things
: Build a mozutils library containing jemalloc and other things
Status: RESOLVED FIXED
fixed-in-bs
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Mike Hommey [:glandium]
:
Mentors:
Depends on: 693286 678195 680373 680440 683875 685554 689049 690856 691876 695989 706042
Blocks: 662814 414946 685480 688999
  Show dependency treegraph
 
Reported: 2011-08-09 05:15 PDT by Mike Hommey [:glandium]
Modified: 2011-12-01 00:25 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
imported patch (21.66 KB, patch)
2011-08-19 11:21 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Splinter Review
Build a mozutils library containing jemalloc and other things (36.03 KB, patch)
2011-08-25 12:39 PDT, Mike Hommey [:glandium]
khuey: review+
Details | Diff | Splinter Review
Build a mozutils library containing jemalloc and other things (36.03 KB, patch)
2011-08-31 14:33 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Splinter Review

Description Mike Hommey [:glandium] 2011-08-09 05:15:05 PDT
From bug 662814 comment 26:

So, with the constraints from this API, our build system and our various ways of linking things for the various platforms we target, I think the least worse thing to do would be to have a directory where we'd build a libmozutils library that:
 - includes jemalloc when jemalloc is enabled
 - includes the event log infrastructure from this bug
 - includes what currently is libmozutils on android when building for android
 - is a DSO on win/osx and a static lib on linux (i.e., is whatever jemalloc currently is)

I think doing so as a separate patch (i.e. without the event log infrastructure) first would be better.
An advantage of this approach is that whenever we'll need to add something else that has the same kind of constraints, it will be much easier to handle.
Comment 1 Mike Hommey [:glandium] 2011-08-09 05:31:40 PDT
I'm wondering if we shouldn't fold libmozalloc in there as well.
Comment 2 Ted Mielczarek [:ted.mielczarek] 2011-08-09 07:18:02 PDT
If that works (I know libmozalloc is tricky), then yes, I'd say do it.
Comment 3 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-08-09 07:19:06 PDT
I don't think we want to include libmozalloc into something everything links against.  IIRC it's intentional that only libxul uses it.
Comment 4 Mike Hommey [:glandium] 2011-08-09 07:21:13 PDT
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #3)
> I don't think we want to include libmozalloc into something everything links
> against.  IIRC it's intentional that only libxul uses it.

Components use it as well.
Comment 5 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-08-09 08:54:44 PDT
On android, jemalloc is exposed through mozalloc, and everything links against that.
Comment 6 Mike Hommey [:glandium] 2011-08-19 11:21:08 PDT
Created attachment 554479 [details] [diff] [review]
imported patch

This is WIP.
Comment 7 Mike Hommey [:glandium] 2011-08-25 12:39:23 PDT
Created attachment 555811 [details] [diff] [review]
Build a mozutils library containing jemalloc and other things
Comment 8 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-08-31 10:08:58 PDT
Comment on attachment 555811 [details] [diff] [review]
Build a mozutils library containing jemalloc and other things

>diff --git a/config/config.mk b/config/config.mk
>--- a/config/config.mk
>+++ b/config/config.mk
>@@ -233,23 +233,23 @@ endif # NS_TRACE_MALLOC
> 
> endif # MOZ_DEBUG
> 
> # We don't build a static CRT when building a custom CRT,
> # it appears to be broken. So don't link to jemalloc if
> # the Makefile wants static CRT linking.
> ifeq ($(MOZ_MEMORY)_$(USE_STATIC_LIBS),1_1)
> # Disable default CRT libs and add the right lib path for the linker
>-MOZ_MEMORY_LDFLAGS=
>+MOZ_UTILS_LDFLAGS=
> endif

I wonder if any of this is relevant anymore ...

This actually looks completely reasonable.  I expected something much scarier ;-)
Comment 9 Mike Hommey [:glandium] 2011-08-31 10:17:44 PDT
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #8)
> > # We don't build a static CRT when building a custom CRT,
> > # it appears to be broken. So don't link to jemalloc if
> > # the Makefile wants static CRT linking.
> > ifeq ($(MOZ_MEMORY)_$(USE_STATIC_LIBS),1_1)
> > # Disable default CRT libs and add the right lib path for the linker
> >-MOZ_MEMORY_LDFLAGS=
> >+MOZ_UTILS_LDFLAGS=
> > endif
> 
> I wonder if any of this is relevant anymore ...

It still is, unfortunately. Some places where USE_STATIC_LIBS is defined may not actually require it anymore, though. But some definitely do (I tried a build without that hack. lots of fail)
Comment 10 Mike Hommey [:glandium] 2011-08-31 14:33:53 PDT
Created attachment 557330 [details] [diff] [review]
Build a mozutils library containing jemalloc and other things

Refreshed against m-c, only context changes.
Comment 12 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-09-02 14:48:40 PDT
http://hg.mozilla.org/mozilla-central/rev/e2697c06468f
http://hg.mozilla.org/mozilla-central/rev/8e01bc314326
Comment 13 IU 2011-09-02 19:10:47 PDT
The following tinderbox win32 build is missing jemalloc.dll and will not start:

http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-win32/1314999564/

cset: http://hg.mozilla.org/mozilla-central/rev/e5815c156b6c

I assume this is the bug responsible.
Comment 14 Ed Morley [:emorley] 2011-09-02 20:48:54 PDT
The whole b-s merge backed out of m-c for causing bustage:
http://hg.mozilla.org/mozilla-central/rev/472716252ea3

https://tbpl.mozilla.org/?usebuildbot=1&rev=e5815c156b6c
Comment 15 Mike Hommey [:glandium] 2011-09-02 22:45:18 PDT
A clobber would have fixed it. The problem is that our build system doesn't know everything needs to be relinked, since the libraries to link changed :(
Comment 16 Ed Morley [:emorley] 2011-09-03 02:16:59 PDT
Ok fair enough. Unfortunately at 4am when I'd already spent an hour sorting out the mess that was m-i, then retriggered a load on m-c and waited to see how that went, I didn't really have time to pick through and work out what had broken. I asked on #pymake but everyone else was away apart from Callek who agreed that just backing out the lot was the easiest solution.
Comment 17 Mike Hommey [:glandium] 2011-09-05 23:13:17 PDT
The backout was backed out on b-s
Comment 18 neil@parkwaycc.co.uk 2011-09-17 12:22:29 PDT
Worse still, the backout lost the rename from jemalloc.def to mozutils.def.in which meant that I lost my local changes to jemalloc.def :-(
Comment 19 neil@parkwaycc.co.uk 2011-10-08 07:30:38 PDT
Comment on attachment 557330 [details] [diff] [review]
Build a mozutils library containing jemalloc and other things

>+  dnl On Windows, OSX and OS2, we want to link all our binaries against mozutils
>+  MOZ_UTILS_LDFLAGS='$(call EXPAND_LIBNAME_PATH,mozutils,$(LIBXUL_DIST)/lib)'
So, one of my Windows jemalloc builds has a problem. We set these flags that use two variables only known to the mozilla-central build system.

>     if grep -q '__imp__\{0,1\}free' crtdll.obj; then
But my crtdll.obj doesn not contain __imp__free...
>+      MOZ_UTILS_LDFLAGS='-LIBPATH:$(DIST)/lib -NODEFAULTLIB:msvcrt -NODEFAULTLIB:msvcrtd -NODEFAULTLIB:msvcprt -NODEFAULTLIB:msvcprtd -DEFAULTLIB:mozcrt'
So the flags don't get updated,

>     dnl Also pass this to NSPR/NSS
>+    DLLFLAGS="$DLLFLAGS $MOZ_UTILS_LDFLAGS"
And broken flags get passed to NSPR/NSS...

Note You need to log in before you can comment on or make changes to this bug.