418865 - turn on profile-guided optimization on fx-win32-tbox

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P1)

Description

[(not currently active) Ted Mielczarek]

Patch shortly.

Comment 1

[(not currently active) Ted Mielczarek]

Attachment: enable pgo on fx-win32-tbox

Comment 2

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

From offline discussions with Damon, RobSayre, and then Ted:

1) This is being proposed for nightly/clobber builds, as well as hourly/incremental builds as well as release builds.

2) There are similar changes required for Mac. Ted is filing a bug for that.

3) This double-compile process will take extra time to cycle a build; seems like linux would be 2x, win32 would be just under 2x. This will need to be communicated to everyone watching Tinderbox.

4) We are not blocked on bug#418896 or bug#418894.

Comment 3

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

setting to P1 as this is needed for beta4.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 304839 [details] [diff] [review]
enable pgo on fx-win32-tbox

Adding rhelmer to see if we can get someone to look at it this weekend ...

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 304839 [details] [diff] [review]
enable pgo on fx-win32-tbox

r+a=beltzner

The actual meat of this was reviewed by bsmedberg in bug 361343, this is a simple two line patch to turn on the generation of the profile script and then flip the switch.

Comment 6

[Robert Sayre]

I turned this on to get immediate feedback.

Checking in CLOBBER;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/win32/CLOBBER,v  <--  CLOBBER
new revision: 1.69; previous revision: 1.68
done
Checking in mozconfig;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/win32/mozconfig,v  <--  mozconfig
new revision: 1.18; previous revision: 1.17
done
Checking in tinder-config.pl;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/win32/tinder-config.pl,v  <--  tinder-config.pl
new revision: 1.28; previous revision: 1.27
done

Comment 7

[Robert Sayre]

I had to back this out because of these errors:
<http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1203809580.1203817613.29625.gz&fulltext=1#err7>

Comment 8

[(not currently active) Ted Mielczarek]

So I see this:
e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\obj-fx-trunk\dist\include\string\nstsubstring.h(213) : error C2220: warning treated as error - no 'executable' file generated
e:\builds\tinderbox\fx-trunk\winnt_5.2_depend\mozilla\obj-fx-trunk\dist\include\string\nstsubstring.h(213) : warning C4952: 'nsACString_internal::Last' : no profile data found in program database 'xul.pgd'

And then:
79421 of 82043 functions (96.8%) were optimized using profile data
448151847 of 459075423 instructions (97.6%) were optimized using profile data
LINK : fatal error LNK1257: code generation failed

All the previous errors are a normal part of compiling the CRT, unfortunately (bug 416125 is filed on that).

I have no idea what those errors are from, and Google isn't being terribly helpful.  This VM didn't run out of memory or something like that, did it?

Comment 9

[Mike Schroepfer]

(In reply to comment #8)

> I have no idea what those errors are from, and Google isn't being terribly
> helpful.  This VM didn't run out of memory or something like that, did it?
> 

That's probably a good guess.  How much ram does the box have?

Comment 10

[Nick Thomas [:nthomas] (UTC+12)]

The RAM allocated to the VM was bumped to 2GB early in February (there was some swapping after WPO was turned on, other win32 VM's have 1GB). The VM management software says that memory usage hasn't gone over 65% in the last day, with time resolution of 5 minutes. There's also up to 1536MB swap space available.

Let us know if you need further info, or someone to monitor the box in realtime.

Comment 11

[(not currently active) Ted Mielczarek]

We may be able to toss -VERBOSE into LDFLAGS, not sure if it will give us any useful info though:
http://msdn2.microsoft.com/en-us/library/wdsk6as6(VS.80).aspx

Comment 12

[(not currently active) Ted Mielczarek]

I think we should try this again with -VERBOSE and see what happens. Because of the impending freeze, we should either
a) Try this on wednesday after the freeze or
b) Get a clone of fx-win32-tbox to play with.

I don't know what 
1) our VM resources look like currently (do we have room to make a clone?), and 2) what Build's resource allocation looks like, since we'd need a few hours of someone's time to tweak the clone so it didn't interfere with the original.

Justin: can you speak to 1), and joduinn, can you speak to 2)?

Comment 13

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

From comment#10, it sounds like the VM should be good enough. 

From comment#7, comment#8, do we know if this has ever worked? Before we go down the path of building more VMs and tweaking build infrastructure, I'm trying to figure out if this is some weird discrepancy between the build machines and developer machines that needs to be debugged? Or if we are trying something here for the first time?

Comment 14

[Robert Sayre]

(In reply to comment #13)
> From comment#10, it sounds like the VM should be good enough. 
> 
> From comment#7, comment#8, do we know if this has ever worked?

Yes, we've done it.

Comment 15

[Nick Thomas [:nthomas] (UTC+12)]

We have capacity on the machine hosting fx-win32-tbox without hitting our limits to preserve performance. mrz also has a tool that can hot-clone a VM, so we can clone without downtime (perhaps just a slight slowdown). So this is doable, but we'd Rob/Coop to sign up for the config, or I can do it tomorrow morning.

Comment 16

[Robert Helmer [:rhelmer]]

Attachment: [checked in] config for fx-win32-tbox clone

* push nightlies to experimental/pgo 
* do not push updates
* do not push symbols

I think that this would be safe to use on a clone of fx-win32-tbox, as long as it has a unique hostname.

Comment 17

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 305597 [details] [diff] [review]
[checked in] config for fx-win32-tbox clone

r+, but should also do
  $BuildTree  = 'MozillaExperimental';
  $BuildNameExtra = 'PGO';

Comment 18

[Robert Helmer [:rhelmer]]

Comment on attachment 305597 [details] [diff] [review]
[checked in] config for fx-win32-tbox clone

Checked into test_pgo branch with Nick's changes:

Checking in tinder-config.pl;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/win32/tinder-config.pl,v  <--  tinder-config.pl
new revision: 1.29.2.1; previous revision: 1.29
done

Comment 19

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Once this VM is cloned (bug#419530), it should show up on MozillaExperimental. 

Ted will use this VM to debug  https://bugzilla.mozilla.org/show_bug.cgi?id=418865#c7 and https://bugzilla.mozilla.org/show_bug.cgi?id=418865#c8.

Comment 20

[(not currently active) Ted Mielczarek]

Just wanted to update, I tried another build on my machine trying to minimize the differences between the nightly mozconfig and mine, and my build was still successful, so there's definitely some difference between my setup and the tinderbox, but I don't know what it is.

Comment 21

[(not currently active) Ted Mielczarek]

Looks like tete (who does custom builds, including PGO sometimes) hit this too:
http://tete009.seesaa.net/article/73848584.html#A73848584

I need a better Japanese translation to figure out if he actually has a workaround though. :)

Comment 22

[(not currently active) Ted Mielczarek]

We may need to disable PGO in sqlite even if we get this working, it sounds like it can cause dataloss:
http://forums.mozillazine.org/viewtopic.php?p=3271149#3271149

Comment 23

[(not currently active) Ted Mielczarek]

kohei translated the pertinent parts of tete's post for me, and apparently he was able to build successfully by adding -wd4624 and -wd4952 to his CFLAGS/CXXFLAGS. I think we should try that on the next pass.

Comment 24

[(not currently active) Ted Mielczarek]

Attachment: enable pgo, take 2

This includes the warning disabling flags mentioned above.

Comment 25

[(not currently active) Ted Mielczarek]

Comment on attachment 306007 [details] [diff] [review]
enable pgo, take 2

Looking to re-land this today.

Comment 26

[(not currently active) Ted Mielczarek]

sayrer has a patch to turn off PGO in a few places where it exposes bugs (sqlite/storage/places), want that first.

Comment 27

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 306007 [details] [diff] [review]
enable pgo, take 2

a1.9b4=beltzner, suggest waiting for all trees to cycle green before landing just to make sure we get a baseline reading.

(sayrer, ted to co-ordinate landing)

Comment 28

[(not currently active) Ted Mielczarek]

Checking in tools/tinderbox-configs/firefox/win32/CLOBBER;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/win32/CLOBBER,v  <--  CLOBBER
new revision: 1.71; previous revision: 1.70
done
Checking in tools/tinderbox-configs/firefox/win32/mozconfig;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/win32/mozconfig,v  <--  mozconfig
new revision: 1.21; previous revision: 1.20
done
Checking in tools/tinderbox-configs/firefox/win32/tinder-config.pl;
/cvsroot/mozilla/tools/tinderbox-configs/firefox/win32/tinder-config.pl,v  <--  tinder-config.pl
new revision: 1.30; previous revision: 1.29
done

Comment 29

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Discovered  this today on fx-win32-tbox, while investigating trunk tree closure. Is this a PGO problem or unrelated?

14:35:00 		* L/ C

[[e-mail] - 2008/02/28 14:40:22]
No pgc files in
objdir/dist/bin, so the second
build is pointless. Kicked
tinderbox.

[[e-mail] - 2008/02/28 14:30:40]
Hung at:
OBJDIR=obj-fx-trunk python
obj-fx-trunk/_profile/pgo/profileserver.py

firefox.exe in process list
but not on taskbar. Killed
firefox.exe:

Application pid: 7328
FAIL Exited with code 1 during
test run
fx-win32-tbox.build.mozilla.org -
- [28/Feb/2008 11:33:33] "GET
/index.html HTTP/1.1" 200 -
fx-win32-tbox.build.mozilla.org -
- [28/Feb/2008 11:33:33] "GET
/quit.js HTTP/1.1" 200 -
fx-win32-tbox.build.mozilla.org -
- [28/Feb/2008 11:33:34] code
404, message File not found
fx-win32-tbox.build.mozilla.org -
- [28/Feb/2008 11:33:34] "GET
/favicon.ico HTTP/1.1" 404 -

It continued to the second
build, which seems wonky.

Comment 30

[Robert Sayre]

(In reply to comment #29)
> Discovered  this today on fx-win32-tbox, while investigating trunk tree
> closure. Is this a PGO problem or unrelated?

It is a bug in the script that it didn't check the error code. See bug 420187.

Comment 31

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

So the build scripts should check for "FAIL Exited with code 1 during test run" ? I'm happy to file a bug in ReleaseEngineering for this, but I believe it means we'll see more frequent tree closures until bug#420187 is fixed...

Comment 32

[Robert Sayre]

bug 421067 is not related. I've seen it on mac.