914596 - (VC12) Support building with Visual C++ 2013

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[(no longer active)]

Tracker bug to support building with Visual C++ 2013.

Comment 1

[Rodrigo Silveira [:rsilveira] [:rodms]]

I was able to get a clean clobber build after applying the patches at bug 918164, bug 892856 and bug 892856. That was on latest fx-team.

It's too early for any conclusion - just built once - but it built ~10min faster than usual (from 73mins to 63mins). Almost 14% improvement, not bad.

Comment 2

[(Away)]

I'm not actively working on this. There's no point in putting VS2013 on the build bots without a fix for bug 922441.

Comment 3

[Gregory Szorc [:gps]]

According to http://randomascii.wordpress.com/2014/03/22/make-vc-compiles-fast-through-parallel-compilation/, parallel link time code generation (PGO) actually works in VS 2013 and might be able to net us 2-3x PGO link time wins. Look for Ted's comment.

Taras: Are you tracking this as a Windows automation efficiency improvement?

Comment 4

[(dormant account)]

(In reply to Gregory Szorc [:gps] from comment #3)> 
> Taras: Are you tracking this as a Windows automation efficiency improvement?

yes. blocked on 2013 being able to compile us.

Comment 5

[(not currently active) Ted Mielczarek]

I think the only remaining build issue is bug 982310, we're now mostly waiting on bug 914523 to get VS2013 installed on a builder for testing.

Comment 6

[(no longer active)]

Yes, I think we're ready to look into that right now.

Comment 7

[(not currently active) Ted Mielczarek]

(In reply to Gregory Szorc [:gps] from comment #3)
> According to
> http://randomascii.wordpress.com/2014/03/22/make-vc-compiles-fast-through-
> parallel-compilation/, parallel link time code generation (PGO) actually
> works in VS 2013 and might be able to net us 2-3x PGO link time wins. Look
> for Ted's comment.

I finally installed VC2013 and did PGO builds on my local machine with it and VC2010 for comparison. 2013 is *way* faster. A PGO build with 2010 takes 156 minutes locally, and a PGO build with 2013 takes 63 minutes.

Comment 8

[NVD]

http://www.visualstudio.com/en-us/downloads/download-visual-studio-vs#d-visual-studio-2013-update

http://blogs.msdn.com/b/vcblog/archive/2014/08/04/bugs-fixed-in-visual-studio-2013-update-3.aspx

Can Update 3 be installed on the build bots to pick up all the extra fixes from Microsoft?

Comment 9

[(Away)]

(In reply to NVD from comment #8)
> Can Update 3 be installed on the build bots to pick up all the extra fixes
> from Microsoft?

It took us a long time to get our test suite passing with Update 2, and even longer to get it deployed to the build bots. It's difficult to chase a moving target. My inclination would be not to update further unless there is a huge blocking issue (e.g. PGO crash) that's fixed by a newer update.

Comment 10

[(not currently active) Ted Mielczarek]

Yes, let's get this deployed for real, we can worry about updating to a newer version later. It looks like most of those fixed bugs are internal compiler errors, so if we're currently building okay then picking up those fixes doesn't buy us anything right now. (It would be good to have those fixes to avoid future code changes hitting them, but it's not an immediate problem.)

Comment 11

[Emanuel Hoogeveen [:ehoogeveen]]

As a counterpoint, IIRC we never did update the builders to MSVC 2010 SP1 because it was thought that the risk of breaking things in at least one of the active branches was too high. If we don't do the update now, before we officially build with MSVC 2013, it might never happen.

In terms of the bugfixes, the variadic template-related ones look nice; they're the sort of thing that mfbt would use, so whether they can be worked around might depend on both mfbt and consumers. Waldo, do any of them stand out as important to you (see the second link in comment #8)?

Comment 12

[Jeff Walden [:Waldo]]

https://connect.microsoft.com/VisualStudio/feedback/details/812218/internal-compiler-error-while-compiling-incorrect-code-with-std-forward-and-varadic-templates looks (I'm having trouble downloading the attachment with any browser at all) like it describes one common use we would have for variadic templates, when they become available.

https://connect.microsoft.com/VisualStudio/feedback/details/845750/broken-using-template-in-vs2013-update-2-rc also looks potentially interesting, but again without attachment details I can't be sure.

If we update the builders to MSVC 2013 Update 3 sometime before we bump MSVC requirements to 2013, letting this slide longer is probably not unreasonable.  If we can't commit to doing that, it *appears* that a 2013-but-less-than-Update-3 requirement would make variadic templates not useful until we bumped the MSVC requirement to the version past 2013.

Comment 13

[(Away)]

jlund, how much work would it take to push Update 3 to the builders? I'd support updating if it's easy and leaves us plenty of time to stabilize. I wouldn't want this to push us out by another release.

FWIW those issues in comment 12 don't seem too horrible to me. Is the first one valid code? The title suggests not. I wouldn't mind an ICE for that. And the second one lists a straightforward workaround.

Comment 14

[Jeff Walden [:Waldo]]

Re the variadic+forwarding thing, if I'm reading it right, this would be required to replace http://hg.mozilla.org/mozilla-central/annotate/d697d649c765/js/public/Utility.h#l187 with a single method, rather than one overload per number of arguments.  "Horrible" is relative, but I think what we currently have there is fairly bad.

Or, wait.  "Incorrect" code.  I dunno, then, if that matters at all.

Really, I need more information about the problematic patterns they fixed before I can be sure of much here.

Comment 15

[Jordan Lund (:jlund)]

(In reply to David Major [:dmajor] from comment #13)
> jlund, how much work would it take to push Update 3 to the builders? I'd
> support updating if it's easy and leaves us plenty of time to stabilize. I
> wouldn't want this to push us out by another release.
>

we would need to 1) test that vs2010 still works when vs2013 update3 is installed along side it 2) roll it out across every one of our windows machines without any downtime

no hiccups (both 1 and 2 are completely smooth): < 2 weeks

with hiccups: > 1 month

hiccups are hard to foresee. we had issue with vs2010 and cvtres[1] and rollout issues with GPO behaviour[2]. we are also seeing a random build time increase after we installed vs2013[3]. It is sporatic but more frequent. smoking gun is unknown but if we change again, we will want a lot of time to run numerous tests to ensure we don't regress further. 

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1009807#c30
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1019165#c56
[3] https://bugzil.la/1055876

Comment 16

[Jordan Lund (:jlund)]

(In reply to Jordan Lund (:jlund) from comment #15)
> (In reply to David Major [:dmajor] from comment #13)
> > jlund, how much work would it take to push Update 3 to the builders? I'd
> > support updating if it's easy and leaves us plenty of time to stabilize. I
> > wouldn't want this to push us out by another release.
> >
> 
> we would need to 1) test that vs2010 still works when vs2013 update3 is
> installed along side it 2) roll it out across every one of our windows
> machines without any downtime
> 
> no hiccups (both 1 and 2 are completely smooth): < 2 weeks
> 
> with hiccups: > 1 month
> 

good news, markco has finished rolling out vs2013 update 3 across our build machines.

There are two remaining but they have been disabled so we don't have to worry about hitting them on a push: https://bugzilla.mozilla.org/show_bug.cgi?id=1068922#c22

dmajor, this means you can start testing vs2013 again on try.

Comment 17

[(Away)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ea2ea74d0e33

Comment 18

[(Away)]

Oops, comment 17 was meant for bug 1061335. But I guess it's not a bad idea to mention it here!

Comment 19

[(Away)]

We're producing nightlies with VC12 now -- I'd say that counts as support!

Comment 20

[Mike Hommey [:glandium]]

Note, the various removed-files.in were not updated.

Comment 21

[Mike Hommey [:glandium]]

(for the crt)

Comment 22

[(Away)]

Looking at the file comment [0], I don't think the CRT meets any of those criteria (same for mozjs and gkmedias). Am I missing something? Over in bug 1080074 comment 8 I saw that the old CRT is removed automagically.

[0] http://dxr.mozilla.org/mozilla-central/source/browser/installer/removed-files.in#5

Comment 23

[Mike Hommey [:glandium]]

Mmmm interesting (and "rather" new)

Comment 24

[NVD]

http://www.visualstudio.com/news/vs2013-update4-rtm-vs

http://www.microsoft.com/en-us/download/details.aspx?id=44921

Visual Studio 2013 Update 4 (2013.4) RTM released.

Comment 25

[Elbart]

Visual Studio 2013 Community Edition might be relevant too, but it comes with Update 4 by default.
http://www.visualstudio.com/news/vs2013-community-vs

Comment 26

[(not currently active) Ted Mielczarek]

This bug is fixed and these comments aren't really providing any useful information to it. If there are issues building with the latest update they should be filed as new bugs. We're not likely to update the build machines unless there's a compelling reason as it's a lot of work to make that happen.