Closed Bug 1620140 Opened 9 months ago Closed 9 months ago

Convert toolkit/components/telemetry/* to py3

Categories

(Firefox Build System :: General, task, P3)

task

Tracking

(firefox75 fixed)

RESOLVED FIXED
mozilla75
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
No description provided.

: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.

Flags: needinfo?(chutten)

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).

Flags: needinfo?(chutten)
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

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.

You need to log in before you can comment on or make changes to this bug.