Closed Bug 487396 Opened 12 years ago Closed 12 years ago
fix some of the jemalloc windows build madness
When I originally implemented the jemalloc windows CRT build, it was under time pressure and so I settled on a less-than-stellar implementation. preed's patches in bug 429745 fix a lot of the stupid, but they're intermingled with a lot of other stuff to make the debug CRT work. I'm going to separate out just the build hygiene bits and get that landed, since it will make life nicer regardless of whether we support the debug CRT. The primary motivation here is to fix this for pymake instead of using the patch on bug 485404.
This patch does a few things, mostly taken from the patches on bug 429745: 1) Stop setting LIB/PATH, and instead pass -NODEFAULTLIB:msvcrt -LIBPATH:... -DEFAULTLIB:mozcrt19 (+ slight tweak to the CRT patch to rename the import lib we build) 2) Some general cleanup, remove the option to use a pre-built CRT DLL (which I doubt anyone was using anyway) 3) Actually test for exactly VC8SP1 in configure 4) Remove most of the Win32 jemalloc stuff from js/src/configure.in, since it wouldn't really work standalone anyway. Just pass the LDFLAGS down from the top-level configure and use those. 5) sprinkle some more USE_STATIC_LIBS around I haven't done a full build with this patch yet, but it builds jemalloc, nspr, js and xpcom ok, so that's a good start.
Comment on attachment 371656 [details] [diff] [review] clean up some build hacks Builds fine for me on Windows.
Attachment #371656 - Flags: review?(benjamin)
Comment on attachment 371656 [details] [diff] [review] clean up some build hacks I'm a little concerned: how do the jemalloc cflags get passed down to NSPR?
Attachment #371656 - Flags: review?(benjamin) → review+
In DLLFLAGS, just like before, which is why it gets exported in configure. (There are no CFLAGS, only LDFLAGS.)
In reply to: > 2) Some general cleanup, remove the option to use a pre-built CRT DLL > (which I doubt anyone was using anyway) > 4) Remove most of the Win32 jemalloc stuff from js/src/configure.in, since > it wouldn't really work standalone anyway. Just pass the LDFLAGS down from > the top-level configure and use those. I'm concerned about these changes. The NSS and NSPR developers are trying to, and desire to, be able to build mozcrt19 stand-alone, and use pre-built mozcrt19 in their developer builds of NSS and NSPR on Windows. Most of the developers of NSS and NSPR _NEVER_ build Firefox or Thunderbird, but DO desire to test their own developer builds of NSS+NSPR with FF and TB nightlies built by MoCo. We did that for years, until Mozilla started to use mozcrt19 on Windows. Today, this is a huge problem on Windows because stand-alone developer builds of NSS and NSPR do not use MOZCRT19, while Mozilla's builds of NSS and NSPR (built as part of Firefox) DO use MOZCRT19. Also, the NSS + NSPR developers do NOT all (or even mostly) use Mozilla's package of build tools for Windows. In particular, we use neither MSYS shell nor any of the MSYS utilities. We use MKS. This means that we use source files and shell scripts with Windows line endings, not Unix line endings. This has generally NOT been a problem, since the standard (not MSYS) version of CVS for Windows handles line ending conversions nicely. But it has become a problem for building mozcrt19, because those files assume and require MSYS on Windows. In particular, they assume the use of Unix line endings on Windows. (That problem is slowly pervading Firefox's own use of text files on Windows, I might add.) Also, most of the NSS and NSPR team members don't use Hg at all, because of Hg's "all or nothing" mentality with the repository. (Want a single file? download the ENTIRE repository. SHeesh!) We continue to use cvs exclusively. So, to deal with this problem of out stand-alone builds not working with Mozilla nightlies, the NSS developers (primarily me) have been been working on making it possible to A) CVS checkout mozilla/memory/jemalloc by itself, B) build it "stand alone" and then C) build our stand-alone NSS and NSPR with the built mozcrt19. A was easy, using the upstream (not MSYS) cvs client for windows. B was a chore, due to dealing with the line endings problems, and a few other details. - Use of python is COMPLETELY unnecessary, and I don't do it. - It's not clear which directory names should be put into several of the required shell/make variables. I figured some of them out by trial and error. C is the next step. I fear that the changes proposed in the text I quoted above is actually moving mozcrt19 in the direction of making it harder for the NSS and NSPR teams to achieve their goals with mozcrt19. So, the message is: a) please do NOT make it more difficult to build or use mozcrt19 "stand alone" and b) please do not make building mozcrt19 more dependent on MSYS, and c) please do not abandon the jemalloc sources in CVS.
Nelson, it should be possible to mix NSS-without-jemalloc and mozilla-with-jemalloc. If it's not we have bugs with mismatched allocators that we should just fix. In order to build NSS with jemalloc, you'd just have to pass appropriate linker flags, which would be: -MANIFEST:NO -LIBPATH:"$WIN32_CUSTOM_CRT_DIR" -NODEFAULTLIB:msvcrt -NODEFAULTLIB:msvcrtd -DEFAULTLIB:mozcrt19 Obviously this patch is not going to affect the CVS version of jemalloc, because we don't use CVS any more.
Also, the bits you quoted only affect: 1) building Firefox with a pre-built jemalloc DLL and 2) building spidermonkey, which it seems unlikely that you would care about None of this should affect NSPR or NSS, they will continue to be controlled purely by DLLFLAGS from the Firefox build as they were before.
Benjamin, > it should be possible to mix NSS-without-jemalloc and mozilla-with-jemalloc. SHOULD is the operative word there. But it ISN'T possible, and hasn't been since the advent of mozcrt19. When I attempt it, my browser stays up for rather few minutes at MOST. > If it's not we have bugs with mismatched allocators that we should just fix. Right, LOTS of them in Firefox. It's not acceptable to the NSS developers to have to wait any longer for them to be fixed before we can test with Moz FF nightlies.
Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/7e579ac2ae03
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
*** to the full path to pkg-config. *** Or see http://www.freedesktop.org/software/pkgconfig to get pkg-config. checking for valid optimization flags... yes /e/moz-src/pgo/mozilla-central/configure: test: 14.00.50727.762: integer express ion expected checking for stdint.h... (cached) no checking for inttypes.h... (cached) no checking for sys/int_types.h... (cached) no
Attachment #373639 - Flags: review?(ted.mielczarek)
Comment on attachment 373639 [details] [diff] [review] correct version string comparison Good catch! I'd give this r+ if I was a peer.
Attachment #373639 - Attachment description: followup? → correct version string comparison
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
FWIW, you may want to consider making this emit mozcrt191 (and using that as the appropriate -DEFAULTLIB argument), to annotate the fact that there have been substantive changes to what is branded as the "Mozilla Win32 CRT" since 1.9.0.x shipped. (Or, alternatively, do we set the version information appropriately anywhere?)
What do you mean "substantive changes"? Aside from some fixes in jemalloc (which don't look like much more than bug fixes), we're shipping the same VC8 SP1 CRT we have been. We use the version information that comes with the CRT, 8.0, since I don't think we've changed enough to bother changing that.
Comment on attachment 373639 [details] [diff] [review] correct version string comparison Sorry, my bad. I'll push this fix for you.
Attachment #373639 - Flags: review?(ted.mielczarek) → review+
Pushed followup to m-c: http://hg.mozilla.org/mozilla-central/rev/5ce2bb1c6357
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Please push this to CVS, too.
I haven't even pushed it to 1.9.1. As far as I'm concerned, it's just cleanup, so it's not really necessary on those branches.
Possible the following build error came from these patches? LINK : warning LNK4098: defaultlib 'LIBCMT' conflicts with use of other libs; use /NODEFAULTLIB:library
That's pretty vague. Could you give me more context for that warning?
I get that during the libxul linking phase of the build. VC8SP1. jemalloc enabled.
I'm also seeing this problem; I updated my moz-central tree to latest and started getting the duplicate symbol and bad CRT errors when building fennec for Win32. It's happening at libxul.dll link time.
(In reply to comment #13) > What do you mean "substantive changes"? Aside from some fixes in jemalloc > (which don't look like much more than bug fixes), I think those bug fixes are probably relevant, but also the fact that we build it differently enough now to link specifically against it (and not a fake msvcrt). (Although, I had assumed you took all the .def file goo from the original patch to export the symbols for the .dll; so maybe it doesn't matter, since that part got left behind). This is mostly relevant for people who consume mozcrt (NSS, NSPR, other projects, etc.), and need to know more specific information about what did (or didn't) change between particular releases, especially as it may relate to certification or security issues. We've traditionally been, for lack of a better word, sloppy about this. It would be nice to be cleaner about it because it's important to other people who are using the library.
You need to log in before you can comment on or make changes to this bug.