Closed Bug 977579 Opened 8 years ago Closed 8 years ago

Please schedule linux jit-tests in two chunks

Categories

(Release Engineering :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dminor, Assigned: dminor)

References

Details

Attachments

(3 files, 1 obsolete file)

Due to longish run times on the linux test machines we should schedule the jit-tests in two chunks on linux.
How should we modify the call to the harness for the different chunks?
(In reply to Chris AtLee [:catlee] from comment #1)
> How should we modify the call to the harness for the different chunks?

I'll add two additional jittests suites to the linux mozharness unittest config file that handle the chunking, so on linux the buildbot config.py can call mozharness with --jittest-suite jittest1 and jittest2. I'll have the mozharness patch up shortly for review.

I also have a buildbot-configs patch ready, but I'm happy for someone else to take that over if there better (or more correct) way of handling things.
Status: NEW → ASSIGNED
Assignee: nobody → dminor
This can't be landed until the mozharness patch is running ok in production.

Any feedback on a better to handle this would be great.
Attachment #8383074 - Flags: review?(bhearsum)
Comment on attachment 8383074 [details] [diff] [review]
Schedule jit-test in chunks for linux test machines

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

This patch doesn't work, unfortunately:
Traceback (most recent call last):
  File "/home/bhearsum/Mozilla/checkouts/clean/braindump/buildbot-related/builder_list.py", line 24, in <module>
    execfile(os.path.basename(sys.argv[1]), g)
  File "master.cfg", line 132, in <module>
    BRANCH_UNITTEST_VARS['platforms'])
  File "/home/bhearsum/Mozilla/checkouts/clean/buildbotcustom/misc.py", line 2409, in generateTalosBranchObjects
    for suites_name, suites in branch_config['platforms'][platform][slave_platform][unittest_suites]:
ValueError: too many values to unpack

::: mozilla-tests/config.py
@@ +544,5 @@
> +        'script_path': 'scripts/desktop_unittest.py',
> +        'extra_args': ['--jittest-suite', 'jittest2'],
> +        'blob_upload': True,
> +        'script_maxtime': 7200,
> +    }),

The only thing I can suggest is to have a JITTEST and CHUNKED_JITTEST var instead of putting them in the same list. I'm not too fussy though.

@@ +1788,5 @@
> +    # run in chunks on linux only
> +    if platform in ['linux', 'linux64', 'linux64-asan']:
> +        jittests = JITTEST[1:]
> +    else:
> +        jittests = JITTEST[0]

...because this needs to be a list.

@@ +1801,4 @@
>          # try
>          if slave_platform in BRANCHES['try']['platforms'][platform]:
> +            BRANCHES['try']['platforms'][platform][slave_platform]['opt_unittest_suites'] += jittests 
> +            BRANCHES['try']['platforms'][platform][slave_platform]['debug_unittest_suites'] += jittests 

I think you still need to make copies of the list...
Attachment #8383074 - Flags: review?(bhearsum) → review-
Thanks for catching that. I think this addresses your comments.

I wasn't sure if you intended for me to add a deepcopy when using += on the branches. I'm not sure if it is necessary, but I'm happy to play it safe.
Attachment #8383074 - Attachment is obsolete: true
Attachment #8383225 - Flags: review?(bhearsum)
Comment on attachment 8383225 [details] [diff] [review]
Schedule jit-test in chunks for linux test machines

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

::: mozilla-tests/config.py
@@ +1797,5 @@
>              continue  # Don't use rev3 mini's with this stuff
>          # cedar
>          if slave_platform in BRANCHES['cedar']['platforms'][platform]:
> +            BRANCHES['cedar']['platforms'][platform][slave_platform]['opt_unittest_suites'] += deepcopy(jittests[:])
> +            BRANCHES['cedar']['platforms'][platform][slave_platform]['debug_unittest_suites'] += deepcopy(jittests[:])

[:] already creates a copy, so deepcopy() is unnecessary. r=me if you drop that when you land.
Attachment #8383225 - Flags: review?(bhearsum) → review+
Comment on attachment 8383068 [details] [diff] [review]
Add jittest chunks to linux mozharness config.

sorry for delay!
Attachment #8383068 - Flags: review?(jlund) → review+
(In reply to Jordan Lund (:jlund) from comment #8)
> Comment on attachment 8383068 [details] [diff] [review]
> Add jittest chunks to linux mozharness config.
> 
> sorry for delay!

Thanks, pushed to: https://hg.mozilla.org/build/mozharness/rev/5adfb7aec739
mozharness patch is merged and live in production! :)
(In reply to Ben Hearsum [:bhearsum] from comment #7)
> Comment on attachment 8383225 [details] [diff] [review]
> Schedule jit-test in chunks for linux test machines
> 
> Review of attachment 8383225 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mozilla-tests/config.py
> @@ +1797,5 @@
> >              continue  # Don't use rev3 mini's with this stuff
> >          # cedar
> >          if slave_platform in BRANCHES['cedar']['platforms'][platform]:
> > +            BRANCHES['cedar']['platforms'][platform][slave_platform]['opt_unittest_suites'] += deepcopy(jittests[:])
> > +            BRANCHES['cedar']['platforms'][platform][slave_platform]['debug_unittest_suites'] += deepcopy(jittests[:])
> 
> [:] already creates a copy, so deepcopy() is unnecessary. r=me if you drop
> that when you land.

Thanks, pushed to with deepcopy removed: https://hg.mozilla.org/build/buildbot-configs/rev/f6869a15ef66
According to https://wiki.mozilla.org/ReleaseEngineering/Maintenance#Reconfigs_.2F_Deployments, the buildbot-configs patch was put into production on March 6th.
Small downside: they don't work at all, https://tbpl.mozilla.org/php/getParsedLog.php?id=35972271&tree=Try, "/tools/buildbot/bin/python scripts/scripts/desktop_unittest.py --jittest-suite jittest2 --blob-upload-branch try --download-symbols ondemand" -> "Required config file not set! (use --config-file option)"
I forgot to set the config_files for the new jittest suites.
Attachment #8391143 - Flags: review?(bhearsum)
Attachment #8391143 - Flags: review?(bhearsum) → review+
Live.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.