Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
7106002:
Add the /profile flag to linker options for official builds. (Closed)
|
Created:
9 years, 6 months ago by Sigurður Ásgeirsson Modified:
9 years, 4 months ago CC:
chromium-reviews, laforge Base URL:
svn://chrome-svn/chrome/trunk/src/ Visibility:
Public. |
DescriptionEnable /PROFILE for Release builds and remove it from Debug builds.
Landing this change for rogerm@chromium.org, original review at http://codereview.chromium.org/7541080/.
BUG=None
TEST=Incremental linking works again.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=95878
Patch Set 1 #
Total comments: 2
Patch Set 2 : '' #Messages
Total messages: 10 (0 generated)
PTAL.
What's this for? Kind Regards, Anthony Laforge Technical Program Manager Mountain View, CA On Wed, Jun 1, 2011 at 2:21 PM, <siggi@chromium.org> wrote: > Reviewers: bradnelson, > > Message: > PTAL. > > Description: > Add the /profile flag to linker options for official builds. > This deposits "FIXUPS" in the PDB, which grows the PDB file by 5% or so, > but > does otherwise not the generated binaries. > > BUG=none > TESt=none > > Please review this at http://codereview.chromium.org/7106002/ > > SVN Base: svn://chrome-svn/chrome/trunk/src/ > > Affected files: > M build/internal/release_impl_official.gypi > > > Index: build/internal/release_impl_official.gypi > =================================================================== > --- build/internal/release_impl_official.gypi (revision 87517) > +++ build/internal/release_impl_official.gypi (working copy) > @@ -16,6 +16,9 @@ > }, > 'VCLinkerTool': { > 'LinkTimeCodeGeneration': '1', > + # The /profile flag causes the linker to add a "FIXUP" > + # debug stream to the generated PDB. > + 'Profile': 'true', > }, > }, > } > > >
Hi Anthony, this is for the purposes of allowing Syzygy to instrument and reorder the official release binaries. Siggi On Wed, Jun 1, 2011 at 5:24 PM, Anthony LaForge <laforge@chromium.org>wrote: > What's this for? > > Kind Regards, > > Anthony Laforge > Technical Program Manager > Mountain View, CA > > > > On Wed, Jun 1, 2011 at 2:21 PM, <siggi@chromium.org> wrote: > >> Reviewers: bradnelson, >> >> Message: >> PTAL. >> >> Description: >> Add the /profile flag to linker options for official builds. >> This deposits "FIXUPS" in the PDB, which grows the PDB file by 5% or so, >> but >> does otherwise not the generated binaries. >> >> BUG=none >> TESt=none >> >> Please review this at http://codereview.chromium.org/7106002/ >> >> SVN Base: svn://chrome-svn/chrome/trunk/src/ >> >> Affected files: >> M build/internal/release_impl_official.gypi >> >> >> Index: build/internal/release_impl_official.gypi >> =================================================================== >> --- build/internal/release_impl_official.gypi (revision 87517) >> +++ build/internal/release_impl_official.gypi (working copy) >> @@ -16,6 +16,9 @@ >> }, >> 'VCLinkerTool': { >> 'LinkTimeCodeGeneration': '1', >> + # The /profile flag causes the linker to add a "FIXUP" >> + # debug stream to the generated PDB. >> + 'Profile': 'true', >> }, >> }, >> } >> >> >> >
To verify that this doesn't affect the generated binaries, I've done an
official build of Chromium, with and without the flag.
Here's a comparison of the binary and PDB sizes generated with and without
the /profile flag - executive summary is that the .dll's are identical, save
for time stamp and PDB signature, whereas the PDB has grown by ~5% due to
the extra information in it.
C:\src\chrome-release\src\build>dir Release\chrome.dll
release.noprofile\chrome.dll
Volume in drive C is OS
Volume Serial Number is 489D-E670
Directory of C:\src\chrome-release\src\build\Release
06/01/2011 05:04 PM 25,660,928 chrome.dll
1 File(s) 25,660,928 bytes
Directory of C:\src\chrome-release\src\build\release.noprofile
06/01/2011 02:13 PM 25,660,928 chrome.dll
1 File(s) 25,660,928 bytes
0 Dir(s) 353,209,454,592 bytes free
C:\src\chrome-release\src\build>dir Release\chrome_dll.pdb
release.noprofile\chrome_dll.pdb
Volume in drive C is OS
Volume Serial Number is 489D-E670
Directory of C:\src\chrome-release\src\build\Release
06/01/2011 05:04 PM 256,625,664 chrome_dll.pdb
1 File(s) 256,625,664 bytes
Directory of C:\src\chrome-release\src\build\release.noprofile
06/01/2011 02:13 PM 245,197,824 chrome_dll.pdb
1 File(s) 245,197,824 bytes
0 Dir(s) 353,209,454,592 bytes free
On Wed, Jun 1, 2011 at 5:31 PM, Sigurður Ásgeirsson <siggi@chromium.org>wrote:
> Hi Anthony,
>
> this is for the purposes of allowing Syzygy to instrument and reorder the
> official release binaries.
>
> Siggi
>
>
> On Wed, Jun 1, 2011 at 5:24 PM, Anthony LaForge <laforge@chromium.org>wrote:
>
>> What's this for?
>>
>> Kind Regards,
>>
>> Anthony Laforge
>> Technical Program Manager
>> Mountain View, CA
>>
>>
>>
>> On Wed, Jun 1, 2011 at 2:21 PM, <siggi@chromium.org> wrote:
>>
>>> Reviewers: bradnelson,
>>>
>>> Message:
>>> PTAL.
>>>
>>> Description:
>>> Add the /profile flag to linker options for official builds.
>>> This deposits "FIXUPS" in the PDB, which grows the PDB file by 5% or so,
>>> but
>>> does otherwise not the generated binaries.
>>>
>>> BUG=none
>>> TESt=none
>>>
>>> Please review this at http://codereview.chromium.org/7106002/
>>>
>>> SVN Base: svn://chrome-svn/chrome/trunk/src/
>>>
>>> Affected files:
>>> M build/internal/release_impl_official.gypi
>>>
>>>
>>> Index: build/internal/release_impl_official.gypi
>>> ===================================================================
>>> --- build/internal/release_impl_official.gypi (revision 87517)
>>> +++ build/internal/release_impl_official.gypi (working copy)
>>> @@ -16,6 +16,9 @@
>>> },
>>> 'VCLinkerTool': {
>>> 'LinkTimeCodeGeneration': '1',
>>> + # The /profile flag causes the linker to add a "FIXUP"
>>> + # debug stream to the generated PDB.
>>> + 'Profile': 'true',
>>> },
>>> },
>>> }
>>>
>>>
>>>
>>
>
SGTM, thanks for clarifying. On Wed, Jun 1, 2011 at 2:37 PM, Sigurður Ásgeirsson <siggi@chromium.org>wrote: > To verify that this doesn't affect the generated binaries, I've done an > official build of Chromium, with and without the flag. > Here's a comparison of the binary and PDB sizes generated with and without > the /profile flag - executive summary is that the .dll's are identical, save > for time stamp and PDB signature, whereas the PDB has grown by ~5% due to > the extra information in it. > > C:\src\chrome-release\src\build>dir Release\chrome.dll > release.noprofile\chrome.dll > Volume in drive C is OS > Volume Serial Number is 489D-E670 > > Directory of C:\src\chrome-release\src\build\Release > > 06/01/2011 05:04 PM 25,660,928 chrome.dll > 1 File(s) 25,660,928 bytes > > Directory of C:\src\chrome-release\src\build\release.noprofile > > 06/01/2011 02:13 PM 25,660,928 chrome.dll > 1 File(s) 25,660,928 bytes > 0 Dir(s) 353,209,454,592 bytes free > > C:\src\chrome-release\src\build>dir Release\chrome_dll.pdb > release.noprofile\chrome_dll.pdb > Volume in drive C is OS > Volume Serial Number is 489D-E670 > > Directory of C:\src\chrome-release\src\build\Release > > 06/01/2011 05:04 PM 256,625,664 chrome_dll.pdb > 1 File(s) 256,625,664 bytes > > Directory of C:\src\chrome-release\src\build\release.noprofile > > 06/01/2011 02:13 PM 245,197,824 chrome_dll.pdb > 1 File(s) 245,197,824 bytes > 0 Dir(s) 353,209,454,592 bytes free > > > > On Wed, Jun 1, 2011 at 5:31 PM, Sigurður Ásgeirsson <siggi@chromium.org>wrote: > >> Hi Anthony, >> >> this is for the purposes of allowing Syzygy to instrument and reorder the >> official release binaries. >> >> Siggi >> >> >> On Wed, Jun 1, 2011 at 5:24 PM, Anthony LaForge <laforge@chromium.org>wrote: >> >>> What's this for? >>> >>> Kind Regards, >>> >>> Anthony Laforge >>> Technical Program Manager >>> Mountain View, CA >>> >>> >>> >>> On Wed, Jun 1, 2011 at 2:21 PM, <siggi@chromium.org> wrote: >>> >>>> Reviewers: bradnelson, >>>> >>>> Message: >>>> PTAL. >>>> >>>> Description: >>>> Add the /profile flag to linker options for official builds. >>>> This deposits "FIXUPS" in the PDB, which grows the PDB file by 5% or so, >>>> but >>>> does otherwise not the generated binaries. >>>> >>>> BUG=none >>>> TESt=none >>>> >>>> Please review this at http://codereview.chromium.org/7106002/ >>>> >>>> SVN Base: svn://chrome-svn/chrome/trunk/src/ >>>> >>>> Affected files: >>>> M build/internal/release_impl_official.gypi >>>> >>>> >>>> Index: build/internal/release_impl_official.gypi >>>> =================================================================== >>>> --- build/internal/release_impl_official.gypi (revision 87517) >>>> +++ build/internal/release_impl_official.gypi (working copy) >>>> @@ -16,6 +16,9 @@ >>>> }, >>>> 'VCLinkerTool': { >>>> 'LinkTimeCodeGeneration': '1', >>>> + # The /profile flag causes the linker to add a "FIXUP" >>>> + # debug stream to the generated PDB. >>>> + 'Profile': 'true', >>>> }, >>>> }, >>>> } >>>> >>>> >>>> >>> >> >
FYI, here's some more data. On Wed, Jun 1, 2011 at 5:59 PM, Anthony LaForge <laforge@chromium.org>wrote: > SGTM, thanks for clarifying. > > On Wed, Jun 1, 2011 at 2:37 PM, Sigurður Ásgeirsson <siggi@chromium.org>wrote: > >> To verify that this doesn't affect the generated binaries, I've done an >> official build of Chromium, with and without the flag. >> Here's a comparison of the binary and PDB sizes generated with and without >> the /profile flag - executive summary is that the .dll's are identical, save >> for time stamp and PDB signature, whereas the PDB has grown by ~5% due to >> the extra information in it. >> >> C:\src\chrome-release\src\build>dir Release\chrome.dll >> release.noprofile\chrome.dll >> Volume in drive C is OS >> Volume Serial Number is 489D-E670 >> >> Directory of C:\src\chrome-release\src\build\Release >> >> 06/01/2011 05:04 PM 25,660,928 chrome.dll >> 1 File(s) 25,660,928 bytes >> >> Directory of C:\src\chrome-release\src\build\release.noprofile >> >> 06/01/2011 02:13 PM 25,660,928 chrome.dll >> 1 File(s) 25,660,928 bytes >> 0 Dir(s) 353,209,454,592 bytes free >> >> C:\src\chrome-release\src\build>dir Release\chrome_dll.pdb >> release.noprofile\chrome_dll.pdb >> Volume in drive C is OS >> Volume Serial Number is 489D-E670 >> >> Directory of C:\src\chrome-release\src\build\Release >> >> 06/01/2011 05:04 PM 256,625,664 chrome_dll.pdb >> 1 File(s) 256,625,664 bytes >> >> Directory of C:\src\chrome-release\src\build\release.noprofile >> >> 06/01/2011 02:13 PM 245,197,824 chrome_dll.pdb >> 1 File(s) 245,197,824 bytes >> 0 Dir(s) 353,209,454,592 bytes free >> >> >> >> On Wed, Jun 1, 2011 at 5:31 PM, Sigurður Ásgeirsson <siggi@chromium.org>wrote: >> >>> Hi Anthony, >>> >>> this is for the purposes of allowing Syzygy to instrument and reorder the >>> official release binaries. >>> >>> Siggi >>> >>> >>> On Wed, Jun 1, 2011 at 5:24 PM, Anthony LaForge <laforge@chromium.org>wrote: >>> >>>> What's this for? >>>> >>>> Kind Regards, >>>> >>>> Anthony Laforge >>>> Technical Program Manager >>>> Mountain View, CA >>>> >>>> >>>> >>>> On Wed, Jun 1, 2011 at 2:21 PM, <siggi@chromium.org> wrote: >>>> >>>>> Reviewers: bradnelson, >>>>> >>>>> Message: >>>>> PTAL. >>>>> >>>>> Description: >>>>> Add the /profile flag to linker options for official builds. >>>>> This deposits "FIXUPS" in the PDB, which grows the PDB file by 5% or >>>>> so, but >>>>> does otherwise not the generated binaries. >>>>> >>>>> BUG=none >>>>> TESt=none >>>>> >>>>> Please review this at http://codereview.chromium.org/7106002/ >>>>> >>>>> SVN Base: svn://chrome-svn/chrome/trunk/src/ >>>>> >>>>> Affected files: >>>>> M build/internal/release_impl_official.gypi >>>>> >>>>> >>>>> Index: build/internal/release_impl_official.gypi >>>>> =================================================================== >>>>> --- build/internal/release_impl_official.gypi (revision 87517) >>>>> +++ build/internal/release_impl_official.gypi (working copy) >>>>> @@ -16,6 +16,9 @@ >>>>> }, >>>>> 'VCLinkerTool': { >>>>> 'LinkTimeCodeGeneration': '1', >>>>> + # The /profile flag causes the linker to add a "FIXUP" >>>>> + # debug stream to the generated PDB. >>>>> + 'Profile': 'true', >>>>> }, >>>>> }, >>>>> } >>>>> >>>>> >>>>> >>>> >>> >> >
M-A has offered to review this - we're sort of blocking until we have builds through the official pipeline to work on.
lgtm http://codereview.chromium.org/7106002/diff/1/build/internal/release_impl_off... File build/internal/release_impl_official.gypi (right): http://codereview.chromium.org/7106002/diff/1/build/internal/release_impl_off... build/internal/release_impl_official.gypi:21: 'Profile': 'true', Add a comment along the lines: # /PROFILE is only available only in Enterprise (team development) versions. Simply comment out that line if you have a lower version and what do to an official build.
thanks, committing. http://codereview.chromium.org/7106002/diff/1/build/internal/release_impl_off... File build/internal/release_impl_official.gypi (right): http://codereview.chromium.org/7106002/diff/1/build/internal/release_impl_off... build/internal/release_impl_official.gypi:21: 'Profile': 'true', On 2011/06/02 13:23:02, Marc-Antoine Ruel wrote: > Add a comment along the lines: > # /PROFILE is only available only in Enterprise (team development) versions. > Simply comment out that line if you have a lower version and what do to an > official build. Done.
Change committed as 87607 |
