Convert toolkit/components/telemetry/* to py3
Categories
(Firefox Build System :: General, task, P3)
Tracking
(firefox75 fixed)
| Tracking | Status | |
|---|---|---|
| firefox75 | --- | fixed |
People
(Reporter: mshal, Assigned: mshal)
References
Details
Attachments
(9 files)
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review | |
|
47 bytes,
text/x-phabricator-request
|
Details | Review |
|
Assignee | |
Comment 1•4 years ago
|
||
|
|
Assignee | |
Comment 2•4 years ago
|
||
Depends on D65457
|
|
Assignee | |
Comment 3•4 years ago
|
||
Depends on D65458
|
|
Assignee | |
Comment 4•4 years ago
|
||
Depends on D65459
|
|
Assignee | |
Comment 5•4 years ago
|
||
Depends on D65460
|
|
Assignee | |
Comment 6•4 years ago
|
||
Depends on D65461
|
|
Assignee | |
Comment 7•4 years ago
|
||
Depends on D65462
|
|
Assignee | |
Comment 8•4 years ago
|
||
Depends on D65463
|
|
Assignee | |
Comment 9•4 years ago
|
||
Depends on D65464
|
|
Assignee | |
Comment 10•4 years ago
|
||
:chutten - it looks like you've done lots of reviews in this area. One thing that was problematic in these scripts is that they were relying on the unspecified ordering of python2's dict iteration to write to the output files (things like 'for name, entry in category.iteritems()' or json.dump()). With python3.5 that order is actually random, which is problematic for things like caching. To get a well-defined ordering, iterating through dicts to generate output needs to be sorted first.
Can you confirm that having a well-defined ordering here won't impact other systems? For example, TelemetryProcessEnums.h changes from:
Content = 0,
Extension = 1,
Parent = 2,
Gpu = 3,
Dynamic = 4,
Socket = 5,
Count = 6
to:
Parent = 0,
Content = 1,
Extension = 2,
Gpu = 3,
Dynamic = 4,
Socket = 5,
Count = 6
If this only needs to be internally consistent within mozilla-central it should be fine, but since it's in a directory called "telemetry" I'm concerned there may be other external systems that could be impacted.
|
||
Comment 11•4 years ago
|
||
You are correct. We were not (or at least Should Not Have Been) depending on the order except that it be consistent for a given invocation: process names, probe names, probe types... all of these are transmitted as strings. Anything for which the order matters should already be in an ordered data structure (like labels which are in a list).
|
||
Comment 12•4 years ago
|
||
Pushed by mshal@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/192cd2af3aa4 Convert gen_histogram_data.py to py3; r=firefox-build-system-reviewers,rstewart https://hg.mozilla.org/integration/autoland/rev/3b3be23d7362 Convert gen_histogram_enum.py to py3; r=firefox-build-system-reviewers,rstewart https://hg.mozilla.org/integration/autoland/rev/1a0b370848e8 Convert gen_histogram_phf.py to py3; r=firefox-build-system-reviewers,rstewart https://hg.mozilla.org/integration/autoland/rev/07adb480d013 Convert gen_scalar_data.py to py3; r=firefox-build-system-reviewers,rstewart https://hg.mozilla.org/integration/autoland/rev/8f19e1a5bc8c Convert gen_scalar_enum.py to py3; r=firefox-build-system-reviewers,rstewart https://hg.mozilla.org/integration/autoland/rev/a1cf0ae852de Convert gen_event_data.py to py3; r=firefox-build-system-reviewers,rstewart https://hg.mozilla.org/integration/autoland/rev/1a5e74eb40a5 Convert gen_event_enum.py to py3; r=firefox-build-system-reviewers,rstewart https://hg.mozilla.org/integration/autoland/rev/7ed725ab985e Convert gen_process_enum.py to py3; r=firefox-build-system-reviewers,rstewart https://hg.mozilla.org/integration/autoland/rev/dff5251bbee4 Convert gen_process_data.py to py3; r=firefox-build-system-reviewers,rstewart
|
||
Comment 13•4 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/192cd2af3aa4
https://hg.mozilla.org/mozilla-central/rev/3b3be23d7362
https://hg.mozilla.org/mozilla-central/rev/1a0b370848e8
https://hg.mozilla.org/mozilla-central/rev/07adb480d013
https://hg.mozilla.org/mozilla-central/rev/8f19e1a5bc8c
https://hg.mozilla.org/mozilla-central/rev/a1cf0ae852de
https://hg.mozilla.org/mozilla-central/rev/1a5e74eb40a5
https://hg.mozilla.org/mozilla-central/rev/7ed725ab985e
https://hg.mozilla.org/mozilla-central/rev/dff5251bbee4
|
||
Comment 14•4 years ago
|
||
FWIW, this looks like to be the root cause for visible builds times improvement on Windows by removing a source of cache misses, and likely addressed the 6th point of bug 1618784.
Description
•