Closed Bug 1341504 Opened 3 years ago Closed 3 years ago

Incremental linking is effectively disabled on Windows

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: glandium, Assigned: jya)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Per https://msdn.microsoft.com/en-us/library/4khtbfyf.aspx :

LINK performs a full link if any of the following options are specified:

    Link Incrementally is not selected (/INCREMENTAL:NO)

    /OPT:REF is selected

    /OPT:ICF is selected

    /OPT:LBR is selected

    /ORDER is selected

Per config.mk, we're always using /OPT:REF on win32, and /OPT:REF,ICF on win64.
> Per config.mk, we're always using /OPT:REF on win32, and /OPT:REF,ICF on win64.

"Always" on opt builds, yes. Debug builds are ok -- there's an ifndef MOZ_DEBUG a bit further up.
(In reply to David Major [:dmajor] from comment #1)
> > Per config.mk, we're always using /OPT:REF on win32, and /OPT:REF,ICF on win64.
> 
> "Always" on opt builds, yes. Debug builds are ok -- there's an ifndef
> MOZ_DEBUG a bit further up.

Can confirm that, yes.  I presume we can't do anything about opt builds?
We could disable these options for non-automation builds to make local builds faster, at the expense of making them less like automation. We've already got the `--enable-release` switch that controls the `DEVELOPER_OPTIONS` variable in configure, we could just use that.
BTW, as a data point, Linux builds already do that with ICF (disabling on local builds, enabling on --enable-release).
For my work I often care about ICF results, but I recognize that I am in the minority here. (I suspect I may be in the minority just for building opt at all; most people I talk to seem to stick to dbg.)
I'm building with MOZ_DEBUG_SYMBOLS enabled (because otherwise debugger cannot find symbols) but disabling DEBUG macros (because otherwise built binaries are toooooo slow and virtually unusable.) Does it count "opt builds"?
(In reply to Masatoshi Kimura [:emk] from comment #6)
> I'm building with MOZ_DEBUG_SYMBOLS enabled (because otherwise debugger
> cannot find symbols) but disabling DEBUG macros (because otherwise built
> binaries are toooooo slow and virtually unusable.) Does it count "opt
> builds"?

Depends what your mozconfig looks like to achieve it. That being said, what you're describing is also the default.
(and is an opt build)
I've just filed bug 1346069 for a case where incremental linking ends up being much worse than full linking. Something to keep in mind.
Blocks: 1326328
See Also: → 1396083
Duplicate of this bug: 1396083
Assignee: nobody → jyavenard
Comment on attachment 8903868 [details]
Bug 1341504 - Don't disable incremental linking when optimizations are turned off.

https://reviewboard.mozilla.org/r/175634/#review181570

::: config/config.mk:183
(Diff revision 1)
> +
> +ifdef MOZ_OPTIMIZE
>  ifdef HAVE_64BIT_BUILD
> -OS_LDFLAGS = -DEBUG -OPT:REF,ICF
> +OS_LDFLAGS += -OPT:REF,ICF
>  else
> -OS_LDFLAGS = -DEBUG -OPT:REF
> +OS_LDFLAGS += -OPT:REF

If you don't mind a drive-by... we could remove the bitness ifdef and just use -OPT:REF,ICF everywhere (on 32-bit builds, -OPT:REF and -OPT:REF,ICF ought to be identical).

I've been meaning to do this for a long time but can't be bothered to land on its own, so I've been waiting for a related patch to sneak it into. :-)
Comment on attachment 8903868 [details]
Bug 1341504 - Don't disable incremental linking when optimizations are turned off.

https://reviewboard.mozilla.org/r/175634/#review181570

> If you don't mind a drive-by... we could remove the bitness ifdef and just use -OPT:REF,ICF everywhere (on 32-bit builds, -OPT:REF and -OPT:REF,ICF ought to be identical).
> 
> I've been meaning to do this for a long time but can't be bothered to land on its own, so I've been waiting for a related patch to sneak it into. :-)

sure i can add that.

arent you concerned that the overhead of parsing of few more characters may take all the benefits away ?  :)
Attachment #8903868 - Flags: review?(ted) → review?(mh+mozilla)
Comment on attachment 8903868 [details]
Bug 1341504 - Don't disable incremental linking when optimizations are turned off.

https://reviewboard.mozilla.org/r/175634/#review183630
Attachment #8903868 - Flags: review?(mh+mozilla) → review+
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/817a2c423412
Don't disable incremental linking when optimizations are turned off. r=glandium
https://hg.mozilla.org/mozilla-central/rev/817a2c423412
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
What happened to the discussion about doing this of opt (the default) developer builds as well?
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.