Closed Bug 508861 Opened 10 years ago Closed 10 years ago
[electrolysis] Build/ship the C++ runtime with MSVC/jemalloc
Electrolysis builds need the C++ runtime. For MSVC builds without jemalloc this means shipping msvcp80.dll which comes with MSVC (http://mxr.mozilla.org/mozilla-central/source/build/win32/Makefile.in#53) For MSVC builds with jemalloc I'm not sure what we need to do. I think we'll have to build the C++ runtime with jemalloc. Ted, since you have MSVC8 can you take this?
Urk. This probably means actually touching the CRT build bits, doesn't it?
It might, I don't know. We probably need to name it mozcprt19.dll or something like that, and we probably don't currently.
Ok, tell you what, I'm willing to do the god-awful nmakefile hacking, but I'll probably need some help on the actual hook-jemalloc-into-the-c++-guts bits.
I don't *think* you'll need to hook jemalloc up at all: as long as ::operator new forwards to malloc() and not directly to some Windows function we should be good.
Since the actual CRT diff is no use, here's what I changed there: * Rename sample_p.dll -> mozcpp19.dll (and other related files) * Stop trying to compile stdhndlr.cpp since it references _set_new_handler, which is in a file that apparently we removed from our libc in the previous round of patching
I realized that I forgot to add the new DLL to the packaging manifest.
Comment on attachment 393497 [details] [diff] [review] build mozcpp19.dll, link to it, package it Ted, can we go ahead and land this on m-c (after 1.9.2 branches) instead of keeping the forked copy in Electrolysis and worrying about regressions when that crash-lands? Or, could we keep the _STATIC_CPPLIB bits (not actually using this new C++ stdlib on mozilla-central) but build it, and electrolysis will take the configure.in changes only?
Attachment #393497 - Flags: review?(benjamin) → review+
We can land it on m-c, it just seemed crummy to start shipping an extra 700KB of DLL that we didn't actually need yet.
Yeah... could we build it but not package it? Or at least land the fixes necessary to build it but not actually build it? I don't want Electrolysis to have a different patch than the upcoming ones for VC9, for example... that sounds like a maintenance/merge nightmare.
Going to hold this till bug 416117 gets fixed, since that has a CRT patch for VC9, and I'll need to fix that as well.
Changed my mind, since this bug was breaking a compiled unittest that jorendorff landed that used C++ features. I'll just have to fix up that other patch before it lands. Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/d25e72de30f1
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
No longer depends on: 416117
Resolution: --- → FIXED
http://hg.mozilla.org/mozilla-central/rev/412b0a03c1cf Disable using the static C++ runtime (on Windows-MSVC) because Chromium requires the full version and we're getting duplicate symbols. Just to double-check, should "dnl Statically link the C++ stdlib. We only use this for Breakpad anyway." be removed too?
Target Milestone: --- → mozilla1.9.3a1
Version: unspecified → Trunk
(In reply to comment #12) Disregard that comment (about an e10s changeset.!.): I got confused trying to sort this story out for c-c...
Blanket approval for Lorentz merge to mozilla-1.9.2 a=beltzner for 126.96.36.199 - please make sure to mark status1.9.2:.4-fixed
Merged into 1.9.2 at http://hg.mozilla.org/releases/mozilla-1.9.2/rev/84ba4d805430
You need to log in before you can comment on or make changes to this bug.