Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
71553003:
Make chromium.perf waterfall use official builds on Win. (Closed)
|
Created:
7 years, 1 month ago by tonyg Modified:
7 years, 1 month ago CC:
dtu, chromium-reviews, cmp-cc_chromium.org, ilevy-cc_chromium.org Visibility:
Public. |
DescriptionPatch Set 1 #
Total comments: 12
Patch Set 2 : #
Messages
Total messages: 14 (0 generated)
laforge, jschuh, could you guys make sure I got all the incantations right for win official build? stip, could you please provide the stamp once they are happy? https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... File masters/master.chromium.perf/master.cfg (left): https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... masters/master.chromium.perf/master.cfg:261: 'component=shared_library ' I'm not sure why this was a shared_library before. If we ship 64 bit chromes on win, would they be static or shared? https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... masters/master.chromium.perf/master.cfg:264: 'GYP_MSVS_VERSION': '2012', I assume this is necessary for 64 bit builds and shipping 64-bit chromes would use 2012?
https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... File masters/master.chromium.perf/master.cfg (left): https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... masters/master.chromium.perf/master.cfg:261: 'component=shared_library ' On 2013/11/13 16:16:25, tonyg wrote: > I'm not sure why this was a shared_library before. If we ship 64 bit chromes on > win, would they be static or shared? They should be static for performance tests. We were just trying to cut cycle times since the link is slow. https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... masters/master.chromium.perf/master.cfg:264: 'GYP_MSVS_VERSION': '2012', On 2013/11/13 16:16:25, tonyg wrote: > I assume this is necessary for 64 bit builds and shipping 64-bit chromes would > use 2012? Oddly enough, early versions of VS can't even build x64 Chrome with WPO. They either crash or have codegen errors. https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... File masters/master.chromium.perf/master.cfg (right): https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... masters/master.chromium.perf/master.cfg:258: 'branding=Chrome component=static_library target_arch=x64', Dumb question from me. Is it branding=Chrome or buildtype=Official that triggers WPO?
https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... File masters/master.chromium.perf/master.cfg (left): https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... masters/master.chromium.perf/master.cfg:264: 'GYP_MSVS_VERSION': '2012', On 2013/11/13 18:15:50, Justin Schuh wrote: > On 2013/11/13 16:16:25, tonyg wrote: > > I assume this is necessary for 64 bit builds and shipping 64-bit chromes would > > use 2012? > > Oddly enough, early versions of VS can't even build x64 Chrome with WPO. They > either crash or have codegen errors. Sorry, that should have been "earlier". Point being that only VS 2012 update 3 can actually build x64 Chrome at all.
ready and waiting... https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... File masters/master.chromium.perf/master.cfg (left): https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... masters/master.chromium.perf/master.cfg:235: 'ffmpeg_branding=Chrome ' do we not need this, as the chromium.chrome builds don't have it either?
https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... File masters/master.chromium.perf/master.cfg (left): https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... masters/master.chromium.perf/master.cfg:235: 'ffmpeg_branding=Chrome ' On 2013/11/13 19:57:17, stip wrote: > do we not need this, as the chromium.chrome builds don't have it either? An official build implies these codecs, so they can be removed. https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... masters/master.chromium.perf/master.cfg:261: 'component=shared_library ' On 2013/11/13 18:15:50, Justin Schuh wrote: > On 2013/11/13 16:16:25, tonyg wrote: > > I'm not sure why this was a shared_library before. If we ship 64 bit chromes > on > > win, would they be static or shared? > > They should be static for performance tests. We were just trying to cut cycle > times since the link is slow. Great, thanks for confirming. https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... masters/master.chromium.perf/master.cfg:264: 'GYP_MSVS_VERSION': '2012', On 2013/11/13 18:17:28, Justin Schuh wrote: > On 2013/11/13 18:15:50, Justin Schuh wrote: > > On 2013/11/13 16:16:25, tonyg wrote: > > > I assume this is necessary for 64 bit builds and shipping 64-bit chromes > would > > > use 2012? > > > > Oddly enough, early versions of VS can't even build x64 Chrome with WPO. They > > either crash or have codegen errors. > > Sorry, that should have been "earlier". Point being that only VS 2012 update 3 > can actually build x64 Chrome at all. OK https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... File masters/master.chromium.perf/master.cfg (right): https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... masters/master.chromium.perf/master.cfg:258: 'branding=Chrome component=static_library target_arch=x64', On 2013/11/13 18:15:50, Justin Schuh wrote: > Dumb question from me. Is it branding=Chrome or buildtype=Official that triggers > WPO? Good question. From common.gypi, it seems like you need both. But I thought I was copying this from the right place. Is there another, more canonical master.cfg I should copy from?
https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... File masters/master.chromium.perf/master.cfg (right): https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... masters/master.chromium.perf/master.cfg:258: 'branding=Chrome component=static_library target_arch=x64', On 2013/11/13 20:15:51, tonyg wrote: > On 2013/11/13 18:15:50, Justin Schuh wrote: > > Dumb question from me. Is it branding=Chrome or buildtype=Official that > triggers > > WPO? > > Good question. From common.gypi, it seems like you need both. But I thought I > was copying this from the right place. Is there another, more canonical > master.cfg I should copy from? James or Scott, do you guys know?
On 2013/11/14 02:35:29, tonyg wrote: > https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... > File masters/master.chromium.perf/master.cfg (right): > > https://codereview.chromium.org/71553003/diff/1/masters/master.chromium.perf/... > masters/master.chromium.perf/master.cfg:258: 'branding=Chrome > component=static_library target_arch=x64', > On 2013/11/13 20:15:51, tonyg wrote: > > On 2013/11/13 18:15:50, Justin Schuh wrote: > > > Dumb question from me. Is it branding=Chrome or buildtype=Official that > > triggers > > > WPO? > > > > Good question. From common.gypi, it seems like you need both. But I thought I > > was copying this from the right place. Is there another, more canonical > > master.cfg I should copy from? > > James or Scott, do you guys know? I believe buildtype= is supposed to cover optimization levels, but in my experience buildtype=Official sometimes busterates without branding=. That's purely anecdotal. When doing perf testing, I always set both.
Talked to laforge offline and he got me pointed to the right config to copy. Everything should be good to go now. Stip: I can haz stamp?
lgtm from me, but i have no idea what i'm doing stip, you're our only hope!!!!
checking with people more knowledgeable than I about the chrome-perf bucket situation
It is a period of civil war. Rebel spaceships, striking from a hidden base, have won their first victory against the evil Galactic Empire. During the battle, Rebel spies managed to steal secret plans to the Empire's ultimate weapon, the OFFICIAL WINDOWS PERF BUILD, an armored space station with enough power to destroy an entire planet. Pursued by the Empire's sinister agents, Tony Gentilcore races home aboard his starship, custodian of the lgtms that can save his people and restore freedom to the galaxy....
On 2013/11/14 19:24:43, stip wrote: > It is a period of civil war. > Rebel spaceships, striking > from a hidden base, have won > their first victory against > the evil Galactic Empire. > > During the battle, Rebel > spies managed to steal secret > plans to the Empire's > ultimate weapon, the OFFICIAL > WINDOWS PERF BUILD, an armored > space station with enough power > to destroy an entire planet. > > Pursued by the Empire's > sinister agents, Tony Gentilcore > races home aboard his > starship, custodian of the > lgtms that can save his > people and restore > freedom to the galaxy.... LOL! You are giving dtu@ a run for his money in terms of epic lgtms
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tonyg@chromium.org/71553003/110001
Message was sent while issue was closed.
Change committed as 235211 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
