In this case, the same marker type "CONTENT_FULL_PAINT_TIME" is used in separate places, so it makes sense to put the marker type definition in a common location.
Differential Revision: https://phabricator.services.mozilla.com/D96042
Because it's easy to send markers to the main thread, we don't need to store the main thread id in memory_hooks functions and objects.
Differential Revision: https://phabricator.services.mozilla.com/D96049
In this case, the same marker type "CONTENT_FULL_PAINT_TIME" is used in separate places, so it makes sense to put the marker type definition in a common location.
Differential Revision: https://phabricator.services.mozilla.com/D96042
Because it's easy to send markers to the main thread, we don't need to store the main thread id in memory_hooks functions and objects.
Differential Revision: https://phabricator.services.mozilla.com/D96049
In this case, the same marker type "CONTENT_FULL_PAINT_TIME" is used in separate places, so it makes sense to put the marker type definition in a common location.
Differential Revision: https://phabricator.services.mozilla.com/D96042
The DevTools mochitests on Fission platforms have been promoted to tier 1.
This changeset updates our try presets to increase our coverage of fission platforms.
Differential Revision: https://phabricator.services.mozilla.com/D96407
The fzf-bin repository is now archived and no longer receives new releases.
Releases are published directly to the fzf repository instead.
Differential Revision: https://phabricator.services.mozilla.com/D96238
This adds back part of the code that was removed in bug 1339182,
reformats it with black, adjusts it to make flake8 happy, and converts
it to python 3.
This also adjusts the script after bug 1534003, which changed the
about:buildconfig page title.
Differential Revision: https://phabricator.services.mozilla.com/D95977
Before, on Windows, this resulted in installing the package in the parent environment (not the `virtualenv`). We fix this by passing down the `virtualenv_manager` so linters can install packages they need using that object's helper methods.
Differential Revision: https://phabricator.services.mozilla.com/D95738
This patch sets up unit tests for the perfdocs linter and adds a few simple tests for it which will be expanded on in future patches.
Differential Revision: https://phabricator.services.mozilla.com/D75480
Unique strings are used to encode all markers' 'name' field, SpliceableJSONWriter::UniqueStringElement can be used for that (instead of a caller-provided callback, which was necessary before UniqueJSONStrings was de-duplicated).
Differential Revision: https://phabricator.services.mozilla.com/D95513
Some markers (e.g., Screenshot) use unique strings in their data.
The UniqueJSONStrings used during streaming is attached to the SpliceableJSONWriter, and StreamJSONMarkerData can use pass-through functions UniqueStringProperty() and UniqueStringElement().
Differential Revision: https://phabricator.services.mozilla.com/D95512
Some markers (e.g., GC major/minor/slice) need to splice JSON strings in their data.
So now, instead of JSONWriter, StreamJSONMarkerData functions will be given a mozilla::baseprofiler::SpliceableJSONWriter.
Differential Revision: https://phabricator.services.mozilla.com/D95511
This adds symbol servers from Intel, AMD and NVidia. Unfortunately not all
symbols are available there, we still have to manually download those that
aren't.
Differential Revision: https://phabricator.services.mozilla.com/D95854
This patch changes a few different things:
- The Docker image used for both scrapers are now based off of our standard
Debian 10 image instead of an old Ubuntu one
- Both images were changed to make it easy to work with them when an
interactive task is used
- The python packages used by the scripts had their versions pinned and
cryptographic hashes have been recorded so that they can be verified before
running the task
- The dump_syms version used on Windows was bumped to include fixes that allow
us to scrape public symbols from DLLs and EXEs that have no PDB file available
- The maximum open file limit of the Window scraper was bumped up to cope with
runs where we gather plenty of symbols in one go
- Similarly the maximum number of symultaneous connections to a single symbol
server has been limited to 4 to avoid being throttled
- The macOS Python tools were modified to work with Python 3
Differential Revision: https://phabricator.services.mozilla.com/D95853
PathUtils is a path manipulation component to IOUtils, which is based on
simplified file I/O. This work is part of the larger goal of removing
osfile.jsm et al., ospath.jsm et al., and the entire OS.* namespace, especially
from the startup path.
No equivalent was provided for OS.Path.fromFileURI because it is unused.
Differential Revision: https://phabricator.services.mozilla.com/D95105
This patch adds the basic docs for Talos using the PerfDocs system. It
includes a high-level overview of what Talos is, and how to interact with
it.
Differential Revision: https://phabricator.services.mozilla.com/D95604
This patch upgrade browsertime to v10, enables webdriver-based navigation in browsertime tests, and also disables the Firefox WindowRecorder for a fairer comparison between Chrome and Firefox.
Differential Revision: https://phabricator.services.mozilla.com/D95425
For consistency with `JSONWriter` (which UniqueJSONStrings' functions use), and for added safety and some efficiency, UniqueJSONStrings now takes `Span<const char`> arguments instead of raw pointers to null-terminated strings.
Differential Revision: https://phabricator.services.mozilla.com/D95114
Document the class and methods.
`GetOrAddIndex` is only used internally, so it can be private.
`SpliceStringTableElements` can now only work on rvalue UniqueJSONStrings, this emphasizes that it shouldn't be used anymore after this call.
Differential Revision: https://phabricator.services.mozilla.com/D95113
There is an unfortunate-but-necessary ordering issue that needs to be fixed in
the api.txt file which is causing a lot of one time updates. The ordering of
annotations was not stable (and now it is).
Differential Revision: https://phabricator.services.mozilla.com/D95382
No significant changes here, mostly clarifying the wording in some places and
making sure it's accurate. Also added a section on the access group since
people looking at this tool may not know about those pieces.
Depends on D95394
Differential Revision: https://phabricator.services.mozilla.com/D95395
We can just use the author of the commit as the committer. I don't know why
I used anything different in the first place, but I'd like to remove references
to the graphics-team email address so this seems like a good time to fix it.
Also remove another TODO that I'm probably never going to do.
Differential Revision: https://phabricator.services.mozilla.com/D95394
The logic the `black` and `flake8` linters were using to find the location of the appropriate binaries for linting was wrong in certain cases given how `mach lint` uses subprocesses to batch work. Instead, we allow the option to override the old janky behavior with a known-good path.
Differential Revision: https://phabricator.services.mozilla.com/D95396
The definition of `patch_main()` has behavior that kicks in only on Windows and for Python 2. Unfortunately, not all of our `mach` commands have been migrated to Python 3, so this still matters.
Bug 1654103 replaced the single-quoted strings in this function with double-quoted strings. This should be fine, except that we call into `multiprocessing.forking.main()` with some monkey-patching that is meant to fix a Windows-specific bug (see bug 1316140). We don't do any clever serialization or anything here and we end up just passing that source to `multiprocessing.forking.main()` which aggregates that source code [dumbly](https://github.com/python/cpython/blob/2.7/Lib/multiprocessing/forking.py#L259), wrapping everything in double-quotes again and passing it to `_subprocess.CreateProcess()`, which ends up failing if the source contains strings formatted with double quotes.
We could revert bug 1654103 and exempt this file from linting, but that is overkill given that this file otherwise contains useful stuff. Instead we move everything to another file, exempt that file from linting, and update `util.py` accordingly.
Differential Revision: https://phabricator.services.mozilla.com/D94909
Instead of a repeating timeout of only twice the parent's serialization time + 1s, use that double parent time and multiply it by the number of children, and add the number of seconds from the about:config preference "devtools.performance.recording.child.timeout_s" (still 1s by default).
Differential Revision: https://phabricator.services.mozilla.com/D94955
Moving markers definitions to where they are used means that it's harder
to unit test them from gtests. xpcshell tests have the benefits of exercising
the markers in a more realistic setting. However, we still need to test the
output of the schema. This patch adds a new capability to do a deep equality
check of the schema results. The utility function attempts to make the output
more readable by humans, and suggest how to fix errors. This will hopefully
make it easy for users to update and maintain their markers.
Differential Revision: https://phabricator.services.mozilla.com/D94890
The baseprofiler is required to be in place first for features that are only
implemented in the baseprofiler, such as the MarkerThreadId::MainThread().
This patch adds the initialization, and upates the ProfilerIOInterposeObserver
to use baseprofiler-only features.
The issue before, was that the base profiler was not initialized, and the
mozilla::baseprofiler::profiler_main_thread_id() was incorrectly reporting that
the main thread index was 0.
No tests were changed in this patch, but it does have coverage with:
tools/profiler/tests/xpcshell/test_feature_fileioall.js
This test would fail with just the interposer changes.
Differential Revision: https://phabricator.services.mozilla.com/D94839
This re-works our tests to run all of the branches in the interposer. There is
a bit of a risk that this won't pass on all platforms as there is an allow list
of known operations. However, it's currently only limited to macOS and Linux.
Please note the placement of utility functions in shared-head.js if they were
generally useful beyond the xpcshell tests, and in xpcshell/head.js if the
functions were only useful for the specific FileIO tests.
Differential Revision: https://phabricator.services.mozilla.com/D93850
This commit uses the new Markers 2.0 API for FileIO Markers. I had to
create another option for the MarkerStack class in order to conditionally
capture a backtrace inside of the Macro. Otherwise the macro invocation
failed.
Differential Revision: https://phabricator.services.mozilla.com/D93848
This is to prevent issues with parsing the correct hostname for displaying and adding
exceptions for urls like view-source:.
Differential Revision: https://phabricator.services.mozilla.com/D94421
The new Markers 2.0 code had missed one detail:
Backtraces in markers were serialized as just the `ProfileChunkedBuffer`, which doesn't expose the original thread id like `ProfilerBacktrace` did.
Then when outputting the profile, the marker code would use the marker's thread id (where the marker should be displayed in the frontend, which *could* be different from where the backtrace came) to deserialize and stream the attached marker, and a special check in the streaming code meant that the mismatched id would ignore the stored sample, and the displayed marker would show no stack.
With this patch, when streaming stacks from markers, the given thread id is 0 (an impossible thread id), which indicates that whatever sample is present should be streamed.
`ProfilerBacktrace` doesn't need to store the thread id anymore.
This solves the above problem.
As a bonus, the streaming code now reports the original thread of the sample(s) it found. This could be used in the future, to better show in the frontend that some stacks may come from other threads.
Differential Revision: https://phabricator.services.mozilla.com/D94264
This re-works our tests to run all of the branches in the interposer. There is
a bit of a risk that this won't pass on all platforms as there is an allow list
of known operations. However, it's currently only limited to macOS and Linux.
Please note the placement of utility functions in shared-head.js if they were
generally useful beyond the xpcshell tests, and in xpcshell/head.js if the
functions were only useful for the specific FileIO tests.
Differential Revision: https://phabricator.services.mozilla.com/D93850
This commit uses the new Markers 2.0 API for FileIO Markers. I had to
create another option for the MarkerStack class in order to conditionally
capture a backtrace inside of the Macro. Otherwise the macro invocation
failed.
Differential Revision: https://phabricator.services.mozilla.com/D93848
This is to prevent issues with parsing the correct hostname for displaying and adding
exceptions for urls like view-source:.
Differential Revision: https://phabricator.services.mozilla.com/D94421
These files were omitted from the original patch because reformatting them required some manual intervention in order to avoid breaking unit tests. Generally the `noqa` lines were already there and just needed to be moved from one line to another (due to the reformatting by `black`), but sometimes `black` saw fit to move a bunch of stuff all onto one line, requiring me to introduce new `noqa` lines.
Besides the autoformat by `black` and some manual fixups, this patch contains no other changes.
# ignore-this-changeset
Differential Revision: https://phabricator.services.mozilla.com/D94052
Depends on D94045
Allow-list all Python code in tree for use with the black linter, and re-format all code in-tree accordingly.
To produce this patch I did all of the following:
1. Make changes to tools/lint/black.yml to remove include: stanza and update list of source extensions.
2. Run ./mach lint --linter black --fix
3. Make some ad-hoc manual updates to python/mozbuild/mozbuild/test/configure/test_configure.py -- it has some hard-coded line numbers that the reformat breaks.
4. Make some ad-hoc manual updates to `testing/marionette/client/setup.py`, `testing/marionette/harness/setup.py`, and `testing/firefox-ui/harness/setup.py`, which have hard-coded regexes that break after the reformat.
5. Add a set of exclusions to black.yml. These will be deleted in a follow-up bug (1672023).
# ignore-this-changeset
Differential Revision: https://phabricator.services.mozilla.com/D94045
These files were omitted from the original patch because reformatting them required some manual intervention in order to avoid breaking unit tests. Generally the `noqa` lines were already there and just needed to be moved from one line to another (due to the reformatting by `black`), but sometimes `black` saw fit to move a bunch of stuff all onto one line, requiring me to introduce new `noqa` lines.
Besides the autoformat by `black` and some manual fixups, this patch contains no other changes.
# ignore-this-changeset
Differential Revision: https://phabricator.services.mozilla.com/D94052
Allow-list all Python code in tree for use with the black linter, and re-format all code in-tree accordingly.
To produce this patch I did all of the following:
1. Make changes to tools/lint/black.yml to remove include: stanza and update list of source extensions.
2. Run ./mach lint --linter black --fix
3. Make some ad-hoc manual updates to python/mozbuild/mozbuild/test/configure/test_configure.py -- it has some hard-coded line numbers that the reformat breaks.
4. Make some ad-hoc manual updates to `testing/marionette/client/setup.py`, `testing/marionette/harness/setup.py`, and `testing/firefox-ui/harness/setup.py`, which have hard-coded regexes that break after the reformat.
5. Add a set of exclusions to black.yml. These will be deleted in a follow-up bug (1672023).
# ignore-this-changeset
Differential Revision: https://phabricator.services.mozilla.com/D94045
These files were omitted from the original patch because reformatting them required some manual intervention in order to avoid breaking unit tests. Generally the `noqa` lines were already there and just needed to be moved from one line to another (due to the reformatting by `black`), but sometimes `black` saw fit to move a bunch of stuff all onto one line, requiring me to introduce new `noqa` lines.
Besides the autoformat by `black` and some manual fixups, this patch contains no other changes.
# ignore-this-changeset
Differential Revision: https://phabricator.services.mozilla.com/D94052
Allow-list all Python code in tree for use with the black linter, and re-format all code in-tree accordingly.
To produce this patch I did all of the following:
1. Make changes to tools/lint/black.yml to remove include: stanza and update list of source extensions.
2. Run ./mach lint --linter black --fix
3. Make some ad-hoc manual updates to python/mozbuild/mozbuild/test/configure/test_configure.py -- it has some hard-coded line numbers that the reformat breaks.
4. Add a set of exclusions to black.yml. These will be deleted in a follow-up bug (1672023).
# ignore-this-changeset
Differential Revision: https://phabricator.services.mozilla.com/D94045
This walks the file system to read moz.build files, rather than following the DIRS
traversal. The latter is problematic because many moz.build files that happen to
define manifests could be excluded by the local build config.
Differential Revision: https://phabricator.services.mozilla.com/D94197
These functions will be useful to get the main thread id, or check if we're in it, in some public code (e.g., markers).
Differential Revision: https://phabricator.services.mozilla.com/D93735
Add `static mozilla::MarkerSchemaWriter MarkerTypeDisplay()` for each existing marker type.
Because all markers of a given type now must have the same payload and display schema, the DOM event marker has changed from type=tracing with category=DOMEvent, to its own type=DOMEvent, which is now accepted on the front-end.
Based on c9692715f2/src/profile-logic/marker-schema.js
Differential Revision: https://phabricator.services.mozilla.com/D90658
`AutoArraySchemaWriter::FreeFormElement()` is never used, we can remove it.
When constructed, `AutoArraySchemaWriter` was optionally taking a `UniqueStrings` reference, which was stored as a pointer.
Then `AutoArraySchemaWriter::StringElement()` would do a dangerous `MOZ_RELEASE_ASSERT(mStrings);`.
The class is now split in two:
- `AutoArraySchemaWriter`, which does not deal with strings at all, so we don't need a pointer to `UniqueStrings`.
- `AutoArraySchemaWithStringsWriter` that can also deal with strings, in which we store a (non-null) `UniqueStrings` reference.
This is both:
- Safer, because it's not possible to instantiate the non-string writer and try to write a string.
- More efficient, because we don't need to pass&store a `UniqueStrings` reference/pointer when we don't deal with strings.
Differential Revision: https://phabricator.services.mozilla.com/D93191
This was causing us to generate the build backend on 'tab' completion (since we
were importing the file to parse available options out of the ArgumentParser
defined therein).
Differential Revision: https://phabricator.services.mozilla.com/D92010
It is possible to add options to a NoPayload marker, but stack and inner window ids would be lost because they are normally stored in the payload 'data' JSON object, which doesn't exist for NoPayload markers.
So in this case, we automatically change the marker to a new (internal) "NoPayloadUserData" type, which has a payload in which we can store options.
This is temporary, until bug 1646714 moves these options into the top-level marker JSON object.
Differential Revision: https://phabricator.services.mozilla.com/D93059
It was only meant to be used internally, when the top-level python
configure invoked the js python subconfigure. Now that this doesn't
happen, we can remove the option, and consolidate js_standalone and
building_js, which are now roughly synonyms.
Differential Revision: https://phabricator.services.mozilla.com/D92726
I created a new test file for testing markers in the parent process. It
can be re-used to test a variety of different markers and their payloads
to ensure they are properly being created, and with relevant information.
The idea here is that this tests the entire pipeline, and excercises the
code as an end user of the profiler would.
Differential Revision: https://phabricator.services.mozilla.com/D92457
This is part of the Markers 2.0 work. This payload proved to be a bit ambiguous
when moving to the new marker schema, so it requires an upgrader.
The test is included as the following commit.
Differential Revision: https://phabricator.services.mozilla.com/D92456
This makes it clearer where marker-type-specific payload arguments start, just after the marker type object.
Also improved the main API documentation.
Differential Revision: https://phabricator.services.mozilla.com/D91681
The `category.WithOptions(...)` syntax was a bit strange and difficult to explain.
Now the category and options are separate parameters. Default options can be specified with `MarkerOptions{}` or just `{}`.
As a special case, defaulted-NoPayload functions don't need `<>`, and defaulted-NoPayload functions and macros don't even need `{}` for default options, e.g.:
`profiler_add_marker("name", OTHER); PROFILER_MARKER_UNTYPED("name", OTHER);`
Differential Revision: https://phabricator.services.mozilla.com/D91680
This makes it clearer where marker-type-specific payload arguments start, just after the marker type object.
Also improved the main API documentation.
Differential Revision: https://phabricator.services.mozilla.com/D91681
The `category.WithOptions(...)` syntax was a bit strange and difficult to explain.
Now the category and options are separate parameters. Default options can be specified with `MarkerOptions{}` or just `{}`.
As a special case, defaulted-NoPayload functions don't need `<>`, and defaulted-NoPayload functions and macros don't even need `{}` for default options, e.g.:
`profiler_add_marker("name", OTHER); PROFILER_MARKER_UNTYPED("name", OTHER);`
Differential Revision: https://phabricator.services.mozilla.com/D91680
This also enables py3 linting for testing/web-platform. The external testing/web-platform/tests is excluded from linting.
Differential Revision: https://phabricator.services.mozilla.com/D90744
Programmatically detecting the moz-phab location by looking at pip
egg/dist files is easily subject to breakage if pip changes its internal
file structure, as it did from pip 19 to pip 20.
So, instead, call the (more stable) pip CLI directly and parse the
output.
Differential Revision: https://phabricator.services.mozilla.com/D91199
The idea here is that if someone else reported a bug and it got fixed immediately, but you don't have the fix yet for whatever reason (e.g you haven't pulled to the latest `central` or whatever), then you'll see it here. I've chosen 15 days as the cutoff basically arbitrarily.
Differential Revision: https://phabricator.services.mozilla.com/D91062
Since `install-moz-phab` is meant to simplify the moz-phab setup flow,
automatically prompting for Phabricator credentials removes an otherwise
manual step.
Detecting the "console_script" location of a package in a
cross-platform, virtualenv-supporting and "--user"-supporting way is
tough, and the most consistent solution seems to be to list the package
contents of moz-phab and look for the one that seems to be the entry
point.
Differential Revision: https://phabricator.services.mozilla.com/D90642
This patch makes us trust the TLS whenever we try to determine whether the current
thread is already registered. It also removes assertions that assume that thread IDs
can never be re-used without a proper unregistration of the old thread.
Differential Revision: https://phabricator.services.mozilla.com/D90915
There is only one call site, so I believe it's best to have the marker type there.
I think it's cleaner this way, this marker type doesn't need to be present in a header shared by all users of the profiler.
The only downside is that we cannot unit-test this particular marker type automatically anymore, but I don't think it's strictly needed:
- There are still plenty of tests checking that generic marker types work end-to-end, so we can have some confidence this specific one can do its job.
- If it somehow started to fail, either it would be quickly found that it breaks the front-end, or it wouldn't have any visible effect in which case it's not a big issue.
Follow-up bug 1665810 will instead add a higher-level xpcshell test or mochitest.
Differential Revision: https://phabricator.services.mozilla.com/D90185
On windows, just subprocessing `pip3 ...` was running the mach
virtualenv's pip3 binary, rather than the system's (or user's
virtualenv's) pip3.
Differential Revision: https://phabricator.services.mozilla.com/D90492
In addition to that remove it from the exclude list of the whitespace sanity check assuming that the dos EOL had made it fail.
Differential Revision: https://phabricator.services.mozilla.com/D85552
