775355 - Run rooting analysis builds on try server

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Steve Fink [:sfink] [:s:]]

Forking this bug from bug 647405 because it is more general.

From IRC:

<lsblakk> so it should be easy to turn on a spidermonkey_try project and have that report to try
<lsblakk> however, that means all pushes to try will use those resources on every push
<lsblakk> and that's something for releng to determine the pros/cons of

Ok, good. So here's my specific justification for this:

We are implementing a moving garbage collector. It requires a change to much of the code in the JS engine, and it's something that is easy to accidentally break. Some of this breakage will be caught immediately by the compiler, but to catch as much of the rest as we can we've come up with a dynamic rooting analysis that will check most uses of the modified API at runtime, during the regular JS tests. We need that running regularly and visibly to prevent regressions, so we want it to be visible by default.

But making it visible by default means that it's effectively a "tier 1 test", so it really needs to be available on the try server to avoid the situation where someone pushes to try, gets a fully green run, and then gets backed out when it burns the tree. Not to mention that the developer would then need to figure out the proper flags to explicitly test it on the tryserver, too.

That's the benefit side. The cost argument is that because this is platform-independent and debug-only, we only need it to run on one configuration. So we would only be adding a single linux64-debug build job per push. (linux-debug, or any other debug build, would work just as well.) If there's some way to restrict these builds to only happen when js/src is touched, as seems to be the case with the other spidermonkey jobs, that would be even better.

Note that Gecko code that uses the JSAPI will also need to be updated and kept correct, but we're not there yet and this change does not cover that case.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Enable rooting analysis build/test on try server

Comment 2

[Steve Fink [:sfink] [:s:]]

The patch is my attempt at setting this up for the staging server. I have no way to test it so it may be totally wrong.

Comment 3

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 643662 [details] [diff] [review]
Enable rooting analysis build/test on try server

I'm glad you've forked this off and started a patch for staging.  I'm actually no longer in Release Engineering though and can't do the testing of this patch as we'd require. Best bet would be to ping in #build and see if someone can help you get this staged.

Comment 4

[[e-mail] (:bhearsum)]

Based on the patch, this isn't a Developer Tools bug.

Comment 5

[Ed Morley [:emorley]]

Comment on attachment 643662 [details] [diff] [review]
Enable rooting analysis build/test on try server

> I'm glad you've forked this off and started a patch for staging.  I'm
> actually no longer in Release Engineering though and can't do the testing of
> this patch as we'd require. Best bet would be to ping in #build and see if
> someone can help you get this staged.

Armen would be a good person to take a look at this I believe :-)

Comment 6

[Ed Morley [:emorley]]

Catlee, ping for review on behalf of sfink :-)
(This is showing up on my own 'My Requests' page; trying to clear it out)

Comment 7

[Chris AtLee [:catlee]]

Comment on attachment 643662 [details] [diff] [review]
Enable rooting analysis build/test on try server

Review of attachment 643662 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, you're on the right track here.

My remaining concern is with which try pushes and up getting this job run. As it is, these jobs will be completely independent of try syntax. Every try push whose top-most push touches js/src will get a linux64-debug rootanalysis build, and all other pushes won't.

::: mozilla/config.py
@@ +977,5 @@
> +    'spidermonkey_try': {
> +        'platforms': {
> +            'linux64-debug':  ['rootanalysis'],
> +        },
> +    },

You'll need to copy the 'env' values here for linux64-debug, as well as setting the appropriate 'hgurl', and 'repo_path' keys.

::: mozilla/staging_config.py
@@ +165,5 @@
> +        'disable_tinderbox_mail': True,
> +        'scripts_repo': 'http://hg.mozilla.org/build/tools',
> +        'idle_slaves': 0,
> +        'tinderbox_tree': 'MozillaTest',
> +    },

This looks fine. The 'disable_tinderbox_mail' and 'tinderbox_tree' aren't used any more, so they could be omitted.

You'll also need corresponding changes to production_config.py and preproduction_config.py.

Comment 8

[Ed Morley [:emorley]]

Not my bug... :-)

Comment 9

[Steve Fink [:sfink] [:s:]]

Attachment: Enable rooting analysis build/test on try server

(In reply to Chris AtLee [:catlee] from comment #7)
> Comment on attachment 643662 [details] [diff] [review]
> Enable rooting analysis build/test on try server
> 
> Review of attachment 643662 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Ok, you're on the right track here.
> 
> My remaining concern is with which try pushes and up getting this job run.
> As it is, these jobs will be completely independent of try syntax. Every try
> push whose top-most push touches js/src will get a linux64-debug
> rootanalysis build, and all other pushes won't.

Just remember, you requested this... ;-)

This patch, plus the other one I'll attach in a minute for the buildbotcustom code, makes the spidermonkey try subject to the try server syntax. I've tested it as much as I could figure out how to, and it does appear that submitting a change that touches js/src and requests linux64-debug will trigger the rootanalysis build, and not otherwise. Maybe I'll attach my hacked-together test program too.

Comment 10

[Steve Fink [:sfink] [:s:]]

Attachment: make spidermonkey try builds subject to try chooser syntax

Note that I left in a couple of commented-out lines. I'm not sure whether they are needed or not. They control whether the scheduler restricts itself to hgpoller-triggered builds. I'm not sure if that applies here or not. I probably ought to test it somehow, but I need to get this out there and I don't relish the thought of diving in again when someone might be able to tell me offhand. (I'm already way, way over my head in this stuff!)

Comment 11

[Steve Fink [:sfink] [:s:]]

Warning: these two patches would have to land simultaneously, because I changed the configuration layout of the spidermonkey project section. If that's a problem, I can make misc.py accept either the old or new syntax to allow an easier transition period.

Comment 12

[Steve Fink [:sfink] [:s:]]

Attachment: NOT FOR LANDING - test code

This is my test program, which I just randomly tweaked (together with much pdb tracing) until it seemed to work. It loads in scheduler_master.py, which is why I don't think it's landable.

Comment 13

[Steve Fink [:sfink] [:s:]]

Attachment: Enable rooting analysis build/test on try server

Updating to depend on bug 794339

Comment 14

[Steve Fink [:sfink] [:s:]]

Attachment: make spidermonkey try builds subject to try chooser syntax

Rebase on top of bug 794339

Comment 15

[Steve Fink [:sfink] [:s:]]

Attachment: make spidermonkey try builds subject to try chooser syntax

Whoops, previous version generated duplicate names for spidermonkey builds and bombed on startup.

Comment 16

[Chris AtLee [:catlee]]

Comment on attachment 673056 [details] [diff] [review]
make spidermonkey try builds subject to try chooser syntax

Review of attachment 673056 [details] [diff] [review]:
-----------------------------------------------------------------

which try chooser syntax will turn on these builds?

Comment 17

[Steve Fink [:sfink] [:s:]]

try: -b d -p linux64

These builds are not individually selectable. They trigger whenever a platform defined for the spidermonkey_try build is chosen, and js/src is touched. The available platforms are set in the 'variants' section of the 'spidermonkey_try' project, which currently only contains 'linux64-debug'. (We picked that one because it's what the relevant developers use, and it's a single one to minimize load since the test should be platform-independent.)

Without this patch, we would get what you gave me an r- for on this bug earlier:

> Every try push whose top-most push touches js/src will get a linux64-debug rootanalysis build, and all other pushes won't.

I could additionally add try syntax specifically for selecting these builds, if you'd like, though I'd probably want to do it separately.

Comment 18

[Chris AtLee [:catlee]]

Comment on attachment 673056 [details] [diff] [review]
make spidermonkey try builds subject to try chooser syntax

Review of attachment 673056 [details] [diff] [review]:
-----------------------------------------------------------------

::: misc.py
@@ +2382,5 @@
>              builders.append(builder)
>  
>      def isImportant(change):
> +        #if not isHgPollerTriggered(change, config['hgurl']):
> +        #    return False

please remove these lines if they're not used.

@@ +2396,5 @@
>      # Set up scheduler
> +    extra_args = {}
> +    scheduler_class = None
> +    if config.get('enable_try'):
> +        assert branch == 'try'

why this assertion? we have other try branches (like try-comm-central), and may in the future have something like 'try-beta', etc.

I think it's ok to leave this assertion out.

Comment 19

[Steve Fink [:sfink] [:s:]]

The assertion was a leftover from when I was trying to understand the configuration stuff. Removed.

Comment 20

[Steve Fink [:sfink] [:s:]]

http://hg.mozilla.org/build/buildbot-configs/rev/08009c0d213d

Comment 21

[Steve Fink [:sfink] [:s:]]

http://hg.mozilla.org/build/buildbotcustom/rev/741742b03f80

Comment 22

[[e-mail] (:bhearsum)]

Comment on attachment 670947 [details] [diff] [review]
Enable rooting analysis build/test on try server

I think this was landed awhile ago.

Comment 23

[[e-mail] (:bhearsum)]

In production.