Bug 914596 (VC12)

Support building with Visual C++ 2013

RESOLVED FIXED in mozilla36

Status

()

defect
RESOLVED FIXED
6 years ago
Last year

People

(Reporter: Ehsan, Unassigned)

Tracking

({dev-doc-needed, meta})

Trunk
mozilla36
All
Windows 8
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Tracker bug to support building with Visual C++ 2013.
Depends on: 914567
Depends on: 892856
Depends on: 892859
Depends on: 915102
Depends on: 915973
Assignee: nobody → dmajor
Depends on: 918158
Depends on: 918164
Depends on: 915522
Depends on: 918906
Depends on: 919069
Depends on: 919735
Depends on: 920318
Depends on: 920740
Depends on: 920751
Depends on: 922441
Blocks: 922912
Depends on: 924657
Depends on: 924745
Depends on: 925599
Keywords: meta
OS: Mac OS X → Windows 8
Hardware: x86 → All
Depends on: 928091
No longer depends on: 928091
Depends on: 929834
Blocks: 931554
No longer blocks: 922912
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.
Depends on: 939557
Depends on: 941931
I'm not actively working on this. There's no point in putting VS2013 on the build bots without a fix for bug 922441.
Assignee: dmajor → nobody
Depends on: 958916
Depends on: 965008
Depends on: 973313
Depends on: 980524
Depends on: 982310
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?
Flags: needinfo?(taras.mozilla)
(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.
Flags: needinfo?(taras.mozilla)
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.
Yes, I think we're ready to look into that right now.
(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.
Depends on: 997401
Depends on: 836658
Depends on: 1001332
Depends on: 1004642
Depends on: 945582
Depends on: 1018402
Blocks: 1020362
Depends on: 1023941
Depends on: 1035068
Depends on: 1055541
Depends on: 1057229
Depends on: 1057839
(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.
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.)
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)?
Flags: needinfo?(jwalden+bmo)
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.
Flags: needinfo?(jwalden+bmo)
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.
Flags: needinfo?(jlund)
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.
Depends on: 1060115
Depends on: 1060117
(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
Flags: needinfo?(jlund)
Depends on: 1060848
Blocks: 1061335
No longer depends on: 1060117
(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.
Depends on: 1082314
Oops, comment 17 was meant for bug 1061335. But I guess it's not a bad idea to mention it here!
Blocks: 609976
Blocks: 922912
We're producing nightlies with VC12 now -- I'd say that counts as support!
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Blocks: 1084162
Note, the various removed-files.in were not updated.
(for the crt)
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
Mmmm interesting (and "rather" new)
Blocks: 1086964
Blocks: 1095103
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
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.
No longer blocks: 1086964
You need to log in before you can comment on or make changes to this bug.