Last Comment Bug 653662 - Disable incremental linking
: Disable incremental linking
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: x86 Windows 7
: -- blocker (vote)
: mozilla6
Assigned To: Mark Banner (:standard8)
:
Mentors:
Depends on:
Blocks: 657571 703845 655558 700959
  Show dependency treegraph
 
Reported: 2011-04-28 23:05 PDT by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2011-11-19 02:16 PST (History)
14 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Disable incremental linking on windoze because we're running into a stupid hard-coded size limit (1.91 KB, patch)
2011-04-28 23:08 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Review
Disable incremental linking only on libxul (699 bytes, patch)
2011-05-16 07:31 PDT, Mark Banner (:standard8)
ted: review+
Details | Diff | Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-04-28 23:05:55 PDT
When building http://hg.mozilla.org/users/cjones_mozilla.com/mcmq/file/510547d841e2/648484-noodle against m-c 88d3c5bde0ba, I get this error

LINK : xul.dll not found or not built by the last incremental link; performing full link
   Creating library xul.lib and object xul.exp
LINK : fatal error LNK1210: exceeded internal ILK size limit; link with /INCREMENTAL:NO
c:\Users\cjones\mozilla\mozilla-central\config\rules.mk:1153:0: command 'c:/mozilla-build/python/python2.6.exe ../../../mozilla-central/config/pythonpath.py -I../../config ../../../mozilla-central/config/expandlibs_exec.py --uselist -- link -NOLOGO -DLL -OUT:xul.dll -PDB:xul.pdb -SUBSYSTEM:WINDOWS  dlldeps-xul.obj nsStaticXULComponents.obj nsDllMain.obj dlldeps.obj nsGFXDeps.obj dlldeps-zlib.obj nsUnicharUtils.obj nsBidiUtils.obj nsRDFResource.obj    ./module.res -LARGEADDRESSAWARE -NXCOMPAT -DYNAMICBASE -SAFESEH  -DEBUG -DEBUGTYPE:CV [snip]

There's a lot of junk in my mozconfig, but the relevant non-default bits are

ac_add_options --disable-optimize
ac_add_options --enable-debug
ac_add_options --enable-trace-malloc
ac_add_options --enable-tests
ac_add_options --enable-ipdl-tests
ac_add_options --enable-chrome-format=symlink
ac_add_options --disable-xpcom-fastload
ac_add_options --disable-xpconnect-idispatch
ac_add_options --disable-activex
ac_add_options --disable-activex-scripting
ac_add_options --disable-accessibility

MSDN sez http://social.msdn.microsoft.com/Forums/en-US/vcgeneral/thread/c34d5c37-ca4a-4580-9c7c-4379a8c76d1f/ ; in a nutshell, there's a hard-coded limit of 384MB for these .ilk files, and I'm apparently exceeding it.

This patch compiled and linked last week, and I was able to compile and link other patches today on my windows machine.  Some combination of the above patch and code that's landed in the last week has broken the camel's back.

(I've never gotten an incremental link myself; I think I hit bug 644698.)
Comment 1 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-04-28 23:08:31 PDT
Created attachment 529043 [details] [diff] [review]
Disable incremental linking on windoze because we're running into a stupid hard-coded size limit

I hope someone can think of a way around this bug.  I'm glad to help provide debugging info.  Otherwise, I think we need this patch :/.
Comment 2 Mark Banner (:standard8) 2011-04-29 01:25:26 PDT
Can't we somehow just do this on just the libxul itself?

We have other dlls and binaries that won't be hitting that limit so it seems a shame to lose the capability there (e.g. I think Neil and maybe some devs would like to link mailnews with external API with incremental linking).
Comment 3 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-04-29 03:27:35 PDT
Hmmm, are you hitting this because of --enable-ipdl-tests?  I seem to remember that causing a bunch of code to be added.

Thunderbird/Seamonkey have been linking in all of mailnews with no problems ...
Comment 4 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-04-29 03:37:07 PDT
Also, the .ilk for my debug libxul is only 154 MB.
Comment 5 Ted Mielczarek [:ted.mielczarek] 2011-04-29 05:01:35 PDT
Have you been rebuilding in this objdir? Does the .ilk file just grow over time?
Comment 6 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-04-29 11:45:37 PDT
(In reply to comment #3)
> Hmmm, are you hitting this because of --enable-ipdl-tests?  I seem to remember
> that causing a bunch of code to be added.

--enable-ipdl-tests adds some code.  In exchange, I have a few other things turned off.  That's not really a game we can force devs to play, though.

(In reply to comment #4)
> Also, the .ilk for my debug libxul is only 154 MB.

Odd.  Are you using VS2010 or 2008?

(In reply to comment #5)
> Have you been rebuilding in this objdir? Does the .ilk file just grow over
> time?

Yes, but, qpushing the patch in comment 0 => failed build, qpop => successful build.  So I don't think it's growing over time.
Comment 7 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-04-29 11:48:47 PDT
(In reply to comment #6)
> (In reply to comment #3)
> > Hmmm, are you hitting this because of --enable-ipdl-tests?  I seem to remember
> > that causing a bunch of code to be added.
> --enable-ipdl-tests adds some code.  In exchange, I have a few other things
> turned off.  That's not really a game we can force devs to play, though.

Sure, we don't want to play that game.  I was under the impression that it added *a lot* of code though.  If that's not true, then disregard that.

> (In reply to comment #4)
> > Also, the .ilk for my debug libxul is only 154 MB.
> Odd.  Are you using VS2010 or 2008?

2010.

I'm really opposed to turning this off unless we absolutely have to, and this is the first report of this problem we've seen ... so I'm hoping this is something weird with your setup.
Comment 8 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-04-29 12:21:18 PDT
$ find ipc/ipdl/ -wholename 'ipc/ipdl/P*Test*.obj' -o -wholename 'ipc/ipdl/test/cxx/*.obj' | xargs ls --block-size=1 -s | awk '{sum=sum+$1}END{print sum}'
78177280

(There's a bit more code turned on by --enable-ipdl-tests, but it's peanuts compared to the above.)

So, even if there were one byte in the .ilk per byte of .obj, --enable-ipdl-tests + comment 4 should be well under the 384MB limit.
Comment 9 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-05-04 06:54:14 PDT
Comment on attachment 529043 [details] [diff] [review]
Disable incremental linking on windoze because we're running into a stupid hard-coded size limit

Clearing the review requests here until this gets more investigation or more reports.
Comment 11 Mark Banner (:standard8) 2011-05-08 11:10:39 PDT
Raising to blocker, as this is now affecting both the Thunderbird & SeaMonkey trunk debug boxes. I suspect it is because we've got mailnews being linked into libxul, but we're going to need a fix to this before the next m-c -> m-a merge.

If there's anything useful I can run on our builders, then let me know (comment here or ping me on irc tomorrow).
Comment 12 Serge Gautherie (:sgautherie) 2011-05-08 12:24:20 PDT
(In reply to comment #0)
> Some combination of the above
> patch and code that's landed in the last week has broken the camel's back.

Interestingly, (c-c) bug 655558 regression timeframe is a week later than this bug.


(In reply to comment #2)
> Can't we somehow just do this on just the libxul itself?

(And for Debug (+ Win32, not Win64) builds only, if that's not what the patch is already doing.)


(In reply to comment #8)
> So, even if there were one byte in the .ilk per byte of .obj,
> --enable-ipdl-tests + comment 4 should be well under the 384MB limit.

What is your current .ilk file size?
(Does a clobber help?)


(In reply to comment #10)
> Both TB and SM seem to be getting this on their trunk Windows Leak Test

Confirmed: bug 655558.
Comment 13 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-05-09 09:37:36 PDT
I don't have an .ilk file (MSVC blows it away?).  Clobbering didn't help.
Comment 14 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-05-09 13:43:57 PDT
FTR, disabling trace-malloc in my local debug build didn't work around this bug.
Comment 15 Mark Banner (:standard8) 2011-05-12 03:46:43 PDT
(In reply to comment #14)
> FTR, disabling trace-malloc in my local debug build didn't work around this
> bug.

You can watch the .ilk file growing as the link happens. I've just done this on a 2005 system, and it seemed to stop at 256MB, which would agree with the article you referenced in comment 0.

I'm double-checking a clobber on our builders, but then I think we'll have to forcibly disable it somehow, maybe via the mozconfig.
Comment 16 Justin Wood (:Callek) 2011-05-15 21:39:50 PDT
(In reply to comment #11)
> Raising to blocker, as this is now affecting both the Thunderbird &
> SeaMonkey trunk debug boxes. I suspect it is because we've got mailnews
> being linked into libxul, but we're going to need a fix to this before the
> next m-c -> m-a merge.

We certainly need some solution for the next m-aurora.

The other concern here is that even as more code is added to Gecko we'll hit this even on the Firefox side, with no easy determinism of the developer who first hits this as to what is wrong.
Comment 17 Siddharth Agarwal [:sid0] (inactive) 2011-05-16 07:15:01 PDT
The thread linked to in comment 0 says that 64-bit Windows has double the limit. Might it pay to switch to that instead of disabling incremental linking? Or at least only disabling incremental linking if we're on 32-bit Windows.
Comment 18 Ted Mielczarek [:ted.mielczarek] 2011-05-16 07:15:54 PDT
Oh, that's interesting! cjones: are you on a 32-bit Windows?
Comment 19 Mark Banner (:standard8) 2011-05-16 07:31:53 PDT
Created attachment 532627 [details] [diff] [review]
Disable incremental linking only on libxul

This disables incremental linking only for libxul and if you're on Windows. I've succeeded in doing a debug build compilation on our windows try builder with this.

I don't think it is ideal, but the only alternatives I can see are:

- Switch to a later Visual Studio version
-- This may help our builders (which seem to have hit this after devs), but not the devs who are seeing this in 2010.
-- We'd also need jemalloc, and I doubt getting that on a later VS version will happen before the next merge.
- Fix Bug 590996 and do this via mozconfigs.
-- I'd be prepared to accept this as an alternative, but landing that patch seems to be going no-where, and it doesn't provide an immediate answer for devs who are hitting the issue.
Comment 20 Serge Gautherie (:sgautherie) 2011-05-16 08:23:19 PDT
(In reply to comment #17)

> The thread linked to in comment 0 says that 64-bit Windows has double the
> limit. Might it pay to switch to that instead of disabling incremental

I don't think everyone can be expected to switch to 64bit just yet.

> linking? Or at least only disabling incremental linking if we're on 32-bit
> Windows.

Yeah, as I noted in comment 12.


(In reply to comment #19)

> This disables incremental linking only for libxul and if you're on Windows.

Should (not) we do this for Win32 only, ftb?

> - Switch to a later Visual Studio version
> -- This may help our builders (which seem to have hit this after devs), but
> not the devs who are seeing this in 2010.

I was going to say "do it for VS 2005 only", but if it can already affect VS 2010 users then fine.
Comment 21 Mark Banner (:standard8) 2011-05-16 09:34:48 PDT
(In reply to comment #20)
> (In reply to comment #19)
> 
> > This disables incremental linking only for libxul and if you're on Windows.
> 
> Should (not) we do this for Win32 only, ftb?

I saw that only after writing the first version of the patch, I'd prefer to get feedback from Ted and co if it'll be accepted before going any further.

@Ted: note that bugzilla says Chris is away until the 28th.
Comment 22 Ted Mielczarek [:ted.mielczarek] 2011-05-16 13:01:10 PDT
Comment on attachment 532627 [details] [diff] [review]
Disable incremental linking only on libxul

This sucks, but we'll go with it for now.
Comment 23 Justin Wood (:Callek) 2011-05-16 19:41:36 PDT
(In reply to comment #17)
> The thread linked to in comment 0 says that 64-bit Windows has double the
> limit. Might it pay to switch to that instead of disabling incremental
> linking? Or at least only disabling incremental linking if we're on 32-bit
> Windows.

For what its worth I'll be attempting a build on VC8EE with Win64 as a test without this patch, in the next 2 weeks or so...

So we can have some data points.
Comment 24 neil@parkwaycc.co.uk 2011-05-17 01:39:41 PDT
(In reply to comment #21)
> (In reply to comment #20)
> > (In reply to comment #19)
> > > This disables incremental linking only for libxul and if you're on Windows.
> > Should (not) we do this for Win32 only, ftb?
> I saw that only after writing the first version of the patch
What's actually tipping the ilk over the limit? MOZ_DEBUG is a sledgehammer, so if you do subsequently tweak the condition you might want to consider conditioning on MOZ_DEBUG_SYMBOLS or MOZ_OPTIMIZE instead as appropriate.
Comment 25 Mark Banner (:standard8) 2011-05-17 03:36:16 PDT
Checked in: http://hg.mozilla.org/mozilla-central/rev/ef9c0d8872cc

I've filed bug 657571 on removing this at some stage if we're able to. If folks want to try improvements to the logic, please deal with those in follow-up bugs, if you want to try it out on the Thunderbird try server, here's what I pushed:

* Pre-patch to get broken build: http://hg.mozilla.org/try-comm-central/rev/610ad75e49ab
* With patch to check get fixed build: http://hg.mozilla.org/try-comm-central/rev/c75bb6e2560b
Comment 26 neil@parkwaycc.co.uk 2011-05-19 01:13:58 PDT
(In reply to comment #23)
> For what its worth I'll be attempting a build on VC8EE with Win64 as a test
> without this patch, in the next 2 weeks or so...
VC8x64/Win64, 369MB .ilk file. But my VC8/Win32 build fails without the patch...
Comment 27 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-05-19 02:57:12 PDT
(In reply to comment #18)
> Oh, that's interesting! cjones: are you on a 32-bit Windows?

Yes.
Comment 28 Ted Mielczarek [:ted.mielczarek] 2011-05-19 05:39:40 PDT
Okay, sounds like we can probably make this disabling even tighter then, and only do it on 32-bit host systems.
Comment 29 Mike Kaganski 2011-11-08 21:15:45 PST
Today I hit this bug building with VC 2010 Express. In the toolkit/library/Makefile.in I see the following lines (different from the patch above):

# See bug 653662 - some builders are hitting an internal size limit
# on incremental builds. Disable this for debug builds using VC8/9.
ifeq ($(OS_ARCH),WINNT)
ifeq (,$(filter-out 1400 1500,$(_MSC_VER)))
ifdef MOZ_DEBUG
EXTRA_DSO_LDOPTS += -INCREMENTAL:NO
endif
endif
endif


I had to comment out
#ifeq (,$(filter-out 1400 1500,$(_MSC_VER)))
(and relevant endif, of course) to be able to compile.
Comment 30 Mark Banner (:standard8) 2011-11-08 23:43:51 PST
(In reply to Mike Kaganski from comment #29)
> In the
> toolkit/library/Makefile.in I see the following lines (different from the
> patch above):

In which case, according to blame, you'll be more interested in bug 696627.
Comment 31 Mike Kaganski 2011-11-08 23:45:08 PST
Thank you. I already found it, excuse me for noise.
Comment 32 Serge Gautherie (:sgautherie) 2011-11-09 00:19:02 PST
(In reply to neil@parkwaycc.co.uk from comment #24)
> What's actually tipping the ilk over the limit? MOZ_DEBUG is a sledgehammer,
> so if you do subsequently tweak the condition you might want to consider
> conditioning on MOZ_DEBUG_SYMBOLS or MOZ_OPTIMIZE instead as appropriate.

(In reply to Ted Mielczarek [:ted, :luser] from comment #28)
> only do it on 32-bit host systems.

I filed bug 700959.
Comment 33 Serge Gautherie (:sgautherie) 2011-11-19 01:29:49 PST
(In reply to Serge Gautherie (:sgautherie) from comment #32)
> (In reply to neil@parkwaycc.co.uk from comment #24)
> > What's actually tipping the ilk over the limit? MOZ_DEBUG is a sledgehammer,
> > so if you do subsequently tweak the condition you might want to consider
> > conditioning on MOZ_DEBUG_SYMBOLS or MOZ_OPTIMIZE instead as appropriate.
> 
> I filed bug 700959.

Forwarded to bug 703845.
Comment 34 Serge Gautherie (:sgautherie) 2011-11-19 02:16:21 PST
(In reply to neil@parkwaycc.co.uk from comment #26)
> (In reply to comment #23)
> > For what its worth I'll be attempting a build on VC8EE with Win64 as a test
> > without this patch, in the next 2 weeks or so...
> VC8x64/Win64, 369MB .ilk file. But my VC8/Win32 build fails without the
> patch...

I filed bug 703852.

Note You need to log in before you can comment on or make changes to this bug.