Closed
Bug 418865
Opened 17 years ago
Closed 17 years ago
turn on profile-guided optimization on fx-win32-tbox
Categories
(Release Engineering :: General, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: ted, Assigned: ted)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
nthomas
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
benjamin
:
review+
beltzner
:
approval1.9b4+
|
Details | Diff | Splinter Review |
Patch shortly.
Assignee | ||
Updated•17 years ago
|
Flags: blocking1.9?
Assignee | ||
Comment 1•17 years ago
|
||
Assignee: nobody → ted.mielczarek
Status: NEW → ASSIGNED
Updated•17 years ago
|
Flags: blocking1.9? → blocking1.9+
Priority: -- → P1
Comment 2•17 years ago
|
||
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.
Flags: blocking1.9+ → blocking1.9?
Priority: P1 → --
Updated•17 years ago
|
Priority: -- → P1
Comment 3•17 years ago
|
||
setting to P1 as this is needed for beta4.
Updated•17 years ago
|
Attachment #304839 -
Flags: review?(ccooper)
Comment 4•17 years ago
|
||
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 ...
Attachment #304839 -
Flags: review?(rhelmer)
Comment 5•17 years ago
|
||
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.
Attachment #304839 -
Flags: review?(rhelmer)
Attachment #304839 -
Flags: review?(ccooper)
Attachment #304839 -
Flags: review+
Attachment #304839 -
Flags: approval1.9+
Updated•17 years ago
|
Flags: blocking1.9? → blocking1.9+
Comment 6•17 years ago
|
||
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•17 years ago
|
||
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>
Assignee | ||
Comment 8•17 years ago
|
||
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•17 years ago
|
||
(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•17 years ago
|
||
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.
Assignee | ||
Comment 11•17 years ago
|
||
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
Assignee | ||
Comment 12•17 years ago
|
||
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•17 years ago
|
||
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•17 years ago
|
||
(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•17 years ago
|
||
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•17 years ago
|
||
* 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.
Attachment #305597 -
Flags: review?(nrthomas)
Comment 17•17 years ago
|
||
Comment on attachment 305597 [details] [diff] [review]
[checked in] config for fx-win32-tbox clone
r+, but should also do
$BuildTree = 'MozillaExperimental';
$BuildNameExtra = 'PGO';
Attachment #305597 -
Flags: review?(nrthomas) → review+
Comment 18•17 years ago
|
||
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
Updated•17 years ago
|
Attachment #305597 -
Attachment description: config for fx-win32-tbox clone → [checked in] config for fx-win32-tbox clone
Comment 19•17 years ago
|
||
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.
Assignee | ||
Comment 20•17 years ago
|
||
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.
Assignee | ||
Comment 21•17 years ago
|
||
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. :)
Assignee | ||
Comment 22•17 years ago
|
||
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
Assignee | ||
Comment 23•17 years ago
|
||
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.
Assignee | ||
Comment 24•17 years ago
|
||
This includes the warning disabling flags mentioned above.
Attachment #304839 -
Attachment is obsolete: true
Attachment #306007 -
Flags: review?(benjamin)
Updated•17 years ago
|
Attachment #306007 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 25•17 years ago
|
||
Comment on attachment 306007 [details] [diff] [review]
enable pgo, take 2
Looking to re-land this today.
Attachment #306007 -
Flags: approval1.9b4?
Assignee | ||
Comment 26•17 years ago
|
||
sayrer has a patch to turn off PGO in a few places where it exposes bugs (sqlite/storage/places), want that first.
Comment 27•17 years ago
|
||
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)
Attachment #306007 -
Flags: approval1.9b4? → approval1.9b4+
Assignee | ||
Comment 28•17 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 29•17 years ago
|
||
Discovered this today on fx-win32-tbox, while investigating trunk tree closure. Is this a PGO problem or unrelated?
14:35:00 * L/ C
[nthomas@mozilla.com - 2008/02/28 14:40:22]
No pgc files in
objdir/dist/bin, so the second
build is pointless. Kicked
tinderbox.
[nthomas@mozilla.com - 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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 30•17 years ago
|
||
(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.
Assignee | ||
Updated•17 years ago
|
Status: REOPENED → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
Comment 31•17 years ago
|
||
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•17 years ago
|
||
bug 421067 is not related. I've seen it on mac.
No longer depends on: 421067
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
You need to log in
before you can comment on or make changes to this bug.
Description
•