508861 - [electrolysis] Build/ship the C++ runtime with MSVC/jemalloc

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Benjamin Smedberg]

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?

Comment 1

[(not currently active) Ted Mielczarek]

Urk. This probably means actually touching the CRT build bits, doesn't it?

Comment 2

[Benjamin Smedberg]

It might, I don't know. We probably need to name it mozcprt19.dll or something like that, and we probably don't currently.

Comment 3

[(not currently active) Ted Mielczarek]

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.

Comment 4

[Benjamin Smedberg]

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.

Comment 5

[(not currently active) Ted Mielczarek]

Attachment: build mozcpp19.dll, link to it

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

Comment 6

[(not currently active) Ted Mielczarek]

Attachment: build mozcpp19.dll, link to it, package it

I realized that I forgot to add the new DLL to the packaging manifest.

Comment 7

[Benjamin Smedberg]

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?

Comment 8

[(not currently active) Ted Mielczarek]

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.

Comment 9

[Benjamin Smedberg]

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.

Comment 10

[(not currently active) Ted Mielczarek]

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.

Comment 11

[(not currently active) Ted Mielczarek]

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

Comment 12

[Serge Gautherie (:sgautherie)]

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?

Comment 13

[Serge Gautherie (:sgautherie)]

(In reply to comment #12)

Disregard that comment (about an e10s changeset.!.): I got confused trying to sort this story out for c-c...

Comment 14

[Benjamin Smedberg]

http://hg.mozilla.org/projects/firefox-lorentz/rev/ecac1bc0ff7e

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

Blanket approval for Lorentz merge to mozilla-1.9.2
a=beltzner for 1.9.2.4 - please make sure to mark status1.9.2:.4-fixed

Comment 16

[Benjamin Smedberg]

Merged into 1.9.2 at http://hg.mozilla.org/releases/mozilla-1.9.2/rev/84ba4d805430