Closed Bug 1397180 Opened 6 years ago Closed 6 years ago

Ability to run with heavy profiles

Categories

(Testing :: Talos, enhancement)

Product:
Testing
Component:
Talos
Version:
Version 3
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox58 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox58 --- fixed

People

(Reporter: tarek, Assigned: tarek)

References

Details

Attachments

(1 file, 1 obsolete file)

Bug 1397180 - Ability to run with heavy profiles
59 bytes, text/x-review-board-request
jmaher
: review+
Details 
Add in Talos a heavy profile downloader
This first patch is just for some early feedback on the design, as I need to clean it up, remove some deps like clint to use testing' built in logger etc.

But I am submitting it it's currently working and I want to know if the way I integrated it in Talos/Mozbase is how you would like to see it integrated. I have not migrated the tests from my initial repo yet.
 
One thing I've built that might be place somewhere is the ability to download diffs instead of full archives. Since talos already uses a mechanism to download zip files for some testing, maybe its best not to create an ad-hoc one and reuse the existing one and improve it.
Flags: needinfo?(jmaher)
Attachment #8904929 - Attachment is obsolete: true
Attachment #8904929 - Flags: review?(jmaher)
Comment on attachment 8904925 [details]
Bug 1397180 - Ability to run with heavy profiles

https://reviewboard.mozilla.org/r/176722/#review181720

f+, this looks like a good approach and nothing is standing out as not a good idea, I would like to reserve an official r+ until we get the entire workflow sorted out and some try runs showing the results.  I will also want :ahal to review the changes as well.

I think overall the latest vs full vs diff needs some better documentation and some final agreement.

::: testing/talos/talos.json:17
(Diff revision 2)
>          },
>          "dromaeojs-stylo-e10s": {
>              "talos_options": ["--enable-stylo"],
>              "tests": ["dromaeo_css", "kraken"]
>          },
> +		"hp": {"tests": ["heavy_profile"]},

as a note, adding a new job in here requires buildbot coordination, etc. even for try;  it is best to just edit an existing test with a profile_path and test it locally/try

::: testing/talos/talos/test.py:247
(Diff revision 2)
>              'profile_path', 'xperf_providers', 'xperf_user_providers', 'xperf_stackwalk',
>              'filters', 'preferences', 'extensions', 'setup', 'cleanup',
>              'lower_is_better', 'alert_threshold', 'unit', 'webextensions']
>  
>  
> +

nit: extra line here
Attachment #8904925 - Flags: review?(jmaher) → review+
Flags: needinfo?(jmaher)
Hey Gregory, 

This patch is a client-side change for talos to grab "heavy profiles" (zipped profiles generated by another tool then uploaded on a server)

I made some ad-hoc things in that patch, and I would like to have something that integrates better with the code base.

Since you're working on mach - which probably does a lot of similar things, I was wondering if you could give me some tips!

- is there any ProgressBar equivalent I can use inside testing/mozbase/mozprofile and testing/talos ? 

- I am doing an ad-hoc sha256 checksum verification on the files, do we have a lib already for that somewhere in the tree? I should use - I know that Firefox tarballs are using sha512 but not sure if mach does anything with it.

- I built a diff-based system to reduce the amount of downloads people would do when grabbing an up-to-date profile. I suspect this is a new use case, but would it make sense to have that moved in a specific package in mozilla-central


Thanks a lot
Flags: needinfo?(gps)
In automation, downloads should come from either tooltool or another taskcluster artifact. If tooltool, the tooltool.py module/script should be used. For everything else, we tend to use various helper functions in mozharness for downloading. These handle things like retries, hash validation, etc. Every time we introduce Python code that performs manual HTTP requests, we introduce a class of bugs due to intermittent failures, firewall issues, etc. So I consider the mere presence of this download code in the added heavy profiles code/module a bug. Sadly, I don't think we have a unified download module that handles all this stuff in a robust manner - the wheel is reinvented partially several times throughout the tree.

You'll probably want to change the heavy profile code to take as arguments local archives or directories that already exist. For archive extraction, mozfile (part of mozbase) has some primitives. But tooltool should probably be used for the actual downloading. In automation, you can modify testing/mozharness/mozharness/mozilla/testing/talos.py to do your bidding. It already has tooltool integration. Note that tooltool can also extract archives automatically and has its own caching mechanism that should be "good enough."

For progress bars, there are a few in the tree. There's the dlmanager package (not sure what that does). There's one in pip. I think pytest has one as well. But since CI doesn't run from a full source checkout, I'm not sure if you can rely on these being available. We do want to run all CI from source checkouts. But that's out-of-scope for this bug.
Flags: needinfo?(gps)
Attachment #8904925 - Flags: review?(ahalberstadt)
You can try it with "--profile simple" to grab the heavy profile named "simple" on TaskCluster
Comment on attachment 8904925 [details]
Bug 1397180 - Ability to run with heavy profiles

https://reviewboard.mozilla.org/r/176722/#review189204

I wonder if this would be something useful to have in mozprofile. Definitely something to consider in a follow-up though.

::: testing/talos/talos/cmdline.py:183
(Diff revision 10)
>              help='If given, disable Stylo via Environment variables.')
>      add_arg('--stylo-threads', type=int,
>              dest='stylothreads',
>              help='If given, run Stylo with a certain number of threads')
> -
> +    add_arg('--profile', type=str, default=None,
> +            help="Downloads a profile from TaskCluster and use it")

nit: uses it

::: testing/talos/talos/heavy.py:67
(Diff revision 10)
> +
> +
> +def profile_age(profile_dir, last_modified=None):
> +    if last_modified is None:
> +        last_modified = datetime.datetime.now()
> +    ages = [os.path.getmtime(root) for root, _, _ in os.walk(profile_dir)]

I could be wrong, but I thought the mtime of a directory doesn't get updated when a file under it changes.

Here's another way of doing this with FileFinder (not sure if it's any faster or not though):
http://searchfox.org/mozilla-central/source/tools/tryselect/tasks.py#25
Attachment #8904925 - Flags: review?(ahalberstadt) → review+
Comment on attachment 8904925 [details]
Bug 1397180 - Ability to run with heavy profiles

https://reviewboard.mozilla.org/r/176722/#review189204

I initially did put it in mozprofile, but was not entirely sure if it was a good fit there. Happy to move it around once we've used it and like the way it works

> I could be wrong, but I thought the mtime of a directory doesn't get updated when a file under it changes.
> 
> Here's another way of doing this with FileFinder (not sure if it's any faster or not though):
> http://searchfox.org/mozilla-central/source/tools/tryselect/tasks.py#25

> the mtime of a directory doesn't get updated when a file under it changes

Good catch, thx!
looking over this again, I think all is good- :ahal had a few good finds!
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 2418532ce711 -d dccc79e2b399: rebasing 423130:2418532ce711 "Bug 1397180 - Ability to run with heavy profiles r=ahal,jmaher" (tip)
merging testing/talos/talos/cmdline.py
merging testing/talos/talos/ffsetup.py
merging testing/talos/talos/run_tests.py
warning: conflicts while merging testing/talos/talos/ffsetup.py! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by tziade@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9323d33afd50
Ability to run with heavy profiles r=ahal,jmaher
Backed out for flake8 linting failure attesting/talos/talos/ffsetup.py:102:13 | local variable 'path' is assigned to but never used:

https://hg.mozilla.org/integration/autoland/rev/6304aff270522a06d4bd5f269c2f549c241e1a13

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=9323d33afd502a26f0d496aa9be81109c0371fb0&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=133838782&repo=autoland
> TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/ffsetup.py:102:13 | local variable 'path' is assigned to but never used (F841)
Flags: needinfo?(tarek)
Oops, looks like the final patch missed a line. I'll fix and push again thx
Flags: needinfo?(tarek)
Pushed by tziade@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/65f6d9e77459
Ability to run with heavy profiles r=ahal,jmaher
https://hg.mozilla.org/mozilla-central/rev/65f6d9e77459
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Blocks: 1407398
You need to log in before you can comment on or make changes to this bug.