Closed Bug 433426 Opened 16 years ago Closed 16 years ago

Confirm final CVS trunk->Hg push to mozilla-central

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: mtschrep, Assigned: jst)

References

Details

Attachments

(1 file)

diff between CVS trunk and mozilla-central.
68.12 KB, patch
Details | Diff | Splinter Review 
Before we enable check-ins to mozilla-central we should:

1) Make sure everything in trunk cvs is in mozilla-central
2) Confirm (via diffs posted here) all changes from cvs-trunk (e.g. so we know what's new in mozilla-central)
3) Confirm that all unit and performance tests are clean
Assignee: nobody → jst
Learnt in today's moz2 meeting that jorendorff was already doing some/all of this.
Are we running perf tests on equivalent hardware, or do we need to do separate perf test runs for comparison?  Should that block opening the tree?
No, the talos hardware is not equivalent. I think that as long as we verify that the code is actually the same, we do not need to do a separate baseline.
Assignee: jst → nobody
Assignee: nobody → jst
I've done the diff, and here's the files that you get from CVS when pulling Firefox (MOZ_CO_PROJECT=browser), but are *not* part of mozilla-central:

mozilla/calendar
mozilla/camino.mk
mozilla/configure
mozilla/dbm
mozilla/extensions/cck
mozilla/extensions/cview
mozilla/extensions/datetime
mozilla/extensions/editor
mozilla/extensions/finger
mozilla/extensions/irc
mozilla/extensions/lightning
mozilla/extensions/manticore
mozilla/extensions/mono
mozilla/extensions/penelope
mozilla/extensions/schema-validation
mozilla/extensions/spatialnavigation
mozilla/extensions/sql
mozilla/extensions/sroaming
mozilla/extensions/tasks
mozilla/extensions/tridentprofile
mozilla/extensions/typeaheadfind
mozilla/extensions/venkman
mozilla/extensions/wallet
mozilla/extensions/webdav
mozilla/extensions/webservices
mozilla/extensions/xforms
mozilla/extensions/xmlextras
mozilla/extensions/xml-rpc
mozilla/extensions/xpath-generator
mozilla/mail
mozilla/.mozconfig.mk
mozilla/.mozconfig.out
mozilla/nsprpub
mozilla/README
mozilla/security/coreconf
mozilla/security/dbm
mozilla/security/manager/.nss.checkout
mozilla/security/nss
mozilla/suite

And here's what's in mozilla-central, but *not* included when you pull Firefox from CVS:

mozilla/build/leaktest.py.in
mozilla/client.py
mozilla/.hg
mozilla/.hgignore
mozilla/.hgtags
mozilla/js/narcissus
mozilla/js/src/config
mozilla/js/src/editline
mozilla/js/tests
mozilla/other-licenses/7zstub/seamonkey
mozilla/other-licenses/7zstub/src
mozilla/other-licenses/7zstub/sunbird
mozilla/other-licenses/7zstub/thunderbird
mozilla/other-licenses/bsdiff
mozilla/testing/extensions
mozilla/testing/performance
mozilla/testing/README.txt
mozilla/testing/release
mozilla/testing/sisyphus
mozilla/testing/tests
mozilla/testing/tinderbox-standalone-tests
mozilla/testing/tools
mozilla/tools/build
mozilla/tools/codesighs
mozilla/tools/cross-commit
mozilla/tools/footprint
mozilla/tools/httptester
mozilla/tools/jprof
mozilla/tools/l10n
mozilla/tools/leaky
mozilla/tools/memory
mozilla/tools/page-loader
mozilla/tools/patcher
mozilla/tools/performance
mozilla/tools/rb
mozilla/tools/release
mozilla/tools/relic
mozilla/tools/reorder
mozilla/tools/tests
mozilla/tools/testserver
mozilla/tools/testy
mozilla/tools/trace-malloc
mozilla/tools/update-packaging
mozilla/tools/uuiddeps
mozilla/xpcom/analysis
mozilla/xpcom/tests/static-checker
mozilla/xulrunner

The diff that that's based on excludes 'CVS' and '.cvsignore'. Complete diff coming up...
This is a diff between what you get when you pull from CVS trunk (at ~16:30 today) using MOZ_CO_PROJECT=browser, and what's in mozilla-central (111c20038fa6), with all .cpp files in mozilla/js/src renamed to .c (so that diff will see the difference between the files). The command used was "diff -ru -x CVS -x .cvsignore cvs hg".

I went through every change here, and they're all changes that I believe are intentionally made in mozilla-central, e.g. build system changes, static checker changes or other misc changes to get our testing boxes happy, plus a couple of misc cleanups done by a couple of people.

Given this, I believe we're all set, I don't see any changes here that's not what we want. Jason, if you want to look over the JS changes, feel free, but they're pretty straight forward and look like changes needed simply to get the C++ compiler to like the code.
I didn't think there were supposed to be changes in js/src/*.cpp: the code was supposed to compile as both C & C++; but we have been checking in C++ fixes in mozilla-central before they were approved for CVS: perhaps that caused the problem?
(In reply to comment #3)
> No, the talos hardware is not equivalent. I think that as long as we verify
> that the code is actually the same, we do not need to do a separate baseline.
> 

John O/Alice can we confirm this?  We just want to make sure that the code and build process on mozilla-central didn't accidentally regress performance before we open for check-ins.
For example, something like bug 433873.

I did look over all the changes in JS.  All the differences are OK.  Apart from makefiles, there are three:

diff -r mozilla/js-trunk/mozilla/js/src/jsinvoke.cpp am/mozilla-central-incoming/js/src/jsinvoke.cpp
43c43
< #include "jsinterp.c"
---
> #include "jsinterp.cpp"

Makes sense.

diff -r mozilla/js-trunk/mozilla/js/src/js.cpp am/mozilla-central-incoming/js/src/js.cpp
2531d2530
< typedef enum ScatterStatus ScatterStatus;
2533c2532
< enum ScatterStatus {
---
> typedef enum ScatterStatus {
2537c2536
< };
---
> } ScatterStatus;

This is a C++ compatibility fix.  IIRC, when I checked this in, mozilla-central simply wouldn't build, and CVS trunk was frozen for a beta release.  The change should also have gone into CVS trunk when the tree opened, but apparently I forgot.  :(

diff -r mozilla/js-trunk/mozilla/js/src/jsinterp.cpp am/mozilla-central-incoming/js/src/jsinterp.cpp
866a867,873
> JS_BEGIN_EXTERN_C
> 
> JSObject*
> js_InitNoSuchMethodClass(JSContext *cx, JSObject* obj);
> 
> JS_END_EXTERN_C
> 

Similar story.  I introduced these lines while merging changes from cvs-trunk-mirror to mozilla-central.  Without them, the changes being merged would break the build (with a linker error, due to the files being compiled as C++).  It should have been filed as a bug against trunk (regressing bug 357016), and the changes should have been pushed back to trunk.

So, bottom line, all is well in js/src.  The changes in mozilla-central are good and should be kept; and they're not crucial enough to push back into CVS right now.

[One minor concern, for bclary:  It would be nice to have these two overlooked changes in the SpiderMonkey 1.8 standalone release, whenever and wherever that happens; we made a lot of other changes to support CC=g++ (bug 357016) and it would be a shame for the feature to be broken in such a tiny way.]

Are we ready to open, or do we need a final performance comparison against trunk?  If the latter, who's signed up to do it?
(In reply to comment #10)

I looked into this.

There are basically four kinds of differences between CVS and mozilla-central:

* js/src is now all built as C++, with some minor changes (see comment 9)

* some build system changes (e.g. moving the checkout-related bits of client.mk into client.py; removing OJI and xml-rpc)

* the switch from tinderbox to buildbot, including related changes to the source

* a tiny number of tiny tiny changes, like off-by-default dehydra stuff.

All these changes have been reviewed over and over by multiple people, and no performance regression is expected.  But anything is possible.

A bunch of us on IRC discussed various ways to test for performance regressions, and it seems like the most sensible plan might be to test 1.9 nightlies head-to-head against Moz2 nightlies using Standalone Talos.

(In reply to comment #11)
> A bunch of us on IRC discussed various ways to test for performance
> regressions, and it seems like the most sensible plan might be to test 1.9
> nightlies head-to-head against Moz2 nightlies using Standalone Talos.

Who's doing this, when is it expected to be done, and should waiting for it keep the tree closed?
(In reply to comment #11)
> (In reply to comment #10)
> 
> I looked into this.
> 
> There are basically four kinds of differences between CVS and mozilla-central:
> 
> * js/src is now all built as C++, with some minor changes (see comment 9)
I didnt think changes like this were supposed to have landed yet. How hard would this be to revert?

> * some build system changes (e.g. moving the checkout-related bits of client.mk
> into client.py; removing OJI and xml-rpc)
How would this change performance?

> * the switch from tinderbox to buildbot, including related changes to the
> source
The only changes we know of were to do with how debug+leak builds were done. There is no change to production/optimized code. If you know of other changes, please elaborate.


> * a tiny number of tiny tiny changes, like off-by-default dehydra stuff.
How would this change performance?


I'm asking because the time/effort to takedown/setup some talos machines for this will take a few days. 
Splitting out the "do talos run on 1.9 hardware to compare moz2 perf vs 1.9 perf" topic to a separate bug#434085
Anything left to do here, given that the perf question seems to have been resolved? I heard talk that we'd sync from CVS once more to pick up the RC2 fixes.

Yeah, we'll merge again, but I think we've verified that there aren't unexpected changes that crept into mercurial.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
For posterity, the last merge was done on May 30th to pick up changes in 3.0rc2 build 2.
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: