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
Virtual environments don't allow user installations.
If a developer enters their own virtual environment before running
`mach install-moz-phab`, moz-phab should be installed to that virtual
environment, rather than attempting to (and failing to) install
to the --user environment.
Differential Revision: https://phabricator.services.mozilla.com/D90455
Allows mach commands to define their own glean metrics with the `metrics_path` @CommandProvider parameter.
When `metrics_path` is defined:
* A `metrics` kwarg is provided to the decorated class. This `metrics` handle is a Glean instance, so Glean documentation should be consulted for usage information.
* When `mach doc telemetry` is run, metrics docs will be generated from all the registered metrics files.
Note: there was some consideration between making `metrics_path` a @CommandProvider or @Command parameter.
In the end, @CommandProvider seemed like a better fit because:
* Metrics seem to be more associated with the entire class than a specific command/method. This is because a class represents a "domain", and that domain may have different commands that have overlapping metrics. Accordingly, all the metrics should be defined once as available to the entire class.
* Currently, @Command methods only take parameters that map one-to-one with CLI arguments. It could seem inconsistent to have one exception: the metrics handle
Differential Revision: https://phabricator.services.mozilla.com/D85953
LUL (a Lightweight Unwind Library) performs unwinding on targets that use
Dwarf CFI metadata. As each Linux/Android shared object is mapped into
memory, it reads unwind data from the objects .eh_frame and/or .debug_frame
sections, and from that info produces a set of canned how-to-unwind recipes,
called RuleSets, which are stored in a SecMap object. There is one SecMap
object for each mapped object in the process.
Each RuleSet describes how to do a step of unwinding for some code address
range. Most code address ranges are very short (a few bytes) and so there are
many RuleSets. libxul.so as of Sept 2020 has approaching 4 million RuleSets,
for example. Currently, each is 48 bytes long, and so storing them all
requires considerable space, over 200MB.
This patch reduces the storage requirement almost by a factor of 6. The key
observation is that although there are many RuleSets, almost all of them are
duplicates. libxul.so typically has less than 300 different RuleSets. This
patch exploits that observation using two different compression schemes.
Firstly, it makes sense to store each different RuleSet only once, in a vector
("the dictionary"). Then, instead of storing (for libxul.so) a vector of 4
million 48-byte-sized RuleSets, we store a vector of 4 million triples, of the
form
(code_address, len, dictionary_index)
If `code_address` is 64-bit, and we (entirely reasonably) constrain `len` and
`dictionary_index` to be 32 bits, then a triple takes 16 bytes. This would
give a factor of 3 memory saving, assuming (again reasonably) that the
dictionary's size is insignificant.
Secondly, we observe that (a) all `code_address`es for any specific shared
object (hence, for the associated RuleSet) span at maximum about 120MB, (b)
that the highest observed `dictionary_index` is less than 400, and (c) that
almost all `len` values are less than 2^16. Hence we can represent that
triple as
(32-bit offset_from_RuleSet_base_address, 16-bit len, 16-bit dictionary_index)
For the few `len` values that won't fit into 16 bits, we can chop the range up
into a sequence of 2^16-1 long chunks. This is exceedingly rare in practice.
With this arrangement, each triple is 8 bytes, hence giving the final
compression figure of 6 == 48 / 8.
In the patch, the triple is represented by a new struct, `Extent`.
This scheme is described (more or less) in
https://blog.mozilla.org/jseward/2013/09/03/how-compactly-can-cfiexidx-stack-unwinding-info-be-represented/
and there is background information on the representations at
https://blog.mozilla.org/jseward/2013/08/29/how-fast-can-cfiexidx-based-stack-unwinding-be/
---
Specific changes are:
class RuleSet: fields `mAddr` and `mLen`, which used to specify the address
range to which the RuleSet applied, have been removed. They can no longer be
part of RuleSet because each RuleSet is now stored only once, and referenced
by each address range fragment which uses it. The address information has
instead been moved to ..
struct Extent: this is a new, 8 byte structure, which specifies address
ranges, and indices into the dictionary of RuleSets, as described above.
class SecMap: this holds all the unwind information harvested from a single
Linux/Android shared object.
* Per the description above, the may-contain-duplicates vector of RuleSets,
`mRuleSet`, has been removed. Instead it is replaced by a vector of
`Extent`s, `mExtents`, and the duplicate-free vector of RuleSets,
`mDictionary`, entries in which are referred to from `mExtents`.
* `mDictionary` cannot be efficiently created until we know all the RuleSets
that it will need to contain. Hence, while reading unwind data, a hash
table, `mUniqifier`, is used as an intermediate. This maps RuleSets to
unique integer IDs. Once reading is complete, `mUniqifier` is enumerated in
order of the unique IDs, and the RuleSets are copied into their correct
locations in `mDictionary`. `mUniqifier` is then deleted, and plays no
further role.
In terms of actions, the main changes are:
* SecMap::AddRuleSet: the new RuleSet is looked up in `mUniqifier`, or added
if missing. This generates a dictionary-index for it. This is the core of
the de-duplication process. Also, a new `mExtent` entry is added for the
range.
* SecMap::PrepareRuleSets: this is called once all info has been read, but
before we commence unwinding. The `mExtent`s implied-address-ranges are
sorted, trimmed and generally tidied up. `mDictionary` is created from
`mUniqifier` and the latter is deleted.
Secondary changes:
* SecMap::mSummaryMinAddr and SecMap::mSummaryMaxAddr have been removed and
replaced by `mMapMinAVMA` and `mMapMaxAVMA`.
`mSummaryMinAddr` and `mSummaryMaxAddr` previously held the minimum and
maximum code addresses of any RuleSets in this SecMap. However, computing
them incrementally is no longer possible, and in any case we need to have a
fixed address for the SecMap against which the Extent::offset fields are
based.
Hence we store instead the lowest and highest code addresses for the mapped
text segment that this SecMap covers -- hence `mMapMinAVMA` and
`mMapMaxAVMA`. These are known before we start reading unwind info for this
SecMap, and are guaranteed to be a superset of the range previously
specified by `mSummaryMinAddr` and `mSummaryMaxAddr`. These ranges are
guaranteed not to overlap the ranges of any other SecMap in the system, and
hence can still be used for their intended purpose of binary-searching to
top level collection of SecMaps (which is owned by the one-and-only PriMap).
* Some comments have been cleaned up. Some imprecise uses of the term
"address" have been replaced with the more precise terminology "AVMA"
(Actual Virtual Memory Address). See existing comment at the top of
LulMain.h.
Differential Revision: https://phabricator.services.mozilla.com/D90289
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
In most situations, JSONWriter users already know string lengths (either directly, or through `nsCString` and friends), so we should keep this information through JSONWriter and not recompute it again.
This also allows using JSONWriter with sub-strings (e.g., from a bigger buffer), without having to create null-terminated strings.
Public JSONWriter functions have overloads that accept literal strings.
Differential Revision: https://phabricator.services.mozilla.com/D86192
`MarkerOptions::Set()` returns the same reference type as the object it's invoked on, i.e.: & -> &, and && -> &&.
`MarkerOptions::NAME()` now always returns a reference to a `const` member, so it's clear it cannot be modified (even if the object at hand is not `const`).
`MarkerOptions::NAMERef()` must be used when non-`const` access is needed.
Differential Revision: https://phabricator.services.mozilla.com/D89715
The name `AUTO_PROFILER_MARKER_TEXT` is more consistent with the equivalent non-`AUTO` macro, and similarly arguments have been re-ordered to be the same, i.e.: Name, category&options, text.
The different macros with different argument sets can now be collapsed into one macro, and the optional arguments (timing, inner window id, backtrace) can easily be added to the `MarkerOptions` where needed.
As a bonus, a specific start time can optionally be provided at construction time.
Differential Revision: https://phabricator.services.mozilla.com/D89588
Mostly mechanical changes, with some work needed to convert the different payloads (with optional timestamps, inner window id, and/or backtrace) to the equivalent MarkerOptions.
Differential Revision: https://phabricator.services.mozilla.com/D89587
Mostly mechanical change, with some extra work where non-literal names are provided.
Also, when this is the only profiler call in a file, `#include "GeckoProfiler.h"` can be changed to `#include "mozilla/ProfilerMarkers.h"`.
Differential Revision: https://phabricator.services.mozilla.com/D89415
`mach bootstrap` doesn't guarantee that it will install `pip3`for you since bug 1647814, so this is bad advice. This removes a weird self-referential loop where `mach bootstrap` recommends you run `mach install-moz-phab` which then recommends you run `mach bootstrap`.
Differential Revision: https://phabricator.services.mozilla.com/D89028
This can be run using:
$ ./mach try --preset builds
The build list can be filtered further using '-xq', e.g:
$ ./mach try --preset builds -xq "win"
Or it can be filtered further interactively:
$ ./mach try --preset builds -xi
Finally, tasks can be added to the build list:
$ ./mach try --preset builds -q "'windows 'mochitest"
This gives a T-style push. There is currently no way to both filter builds and
add additional tasks, it needs to be one or the other.
Differential Revision: https://phabricator.services.mozilla.com/D81352
The profiler should guarantee that TLS initializations are done only once (from `profiler_init()`), so there is no need to potentially do it at every TLS access.
Instead, the initialization functions set the TLS states once (to `Initialized` or `Unavailable`, depending on the underlying TLS initialization success), and later accesses to this effectively-read-only flag can be done without thread synchronization.
This reduces the generated code size.
There are also DEBUG-build checks that no accesses are done before initialization; this is theoretically racy, but it's only used to detect too-early accesses in DEBUG builds, which should never happen anyway.
Differential Revision: https://phabricator.services.mozilla.com/D89338
`ProfileChunkedBuffer` needed to be fully defined, because its destructor is needed to define `UniquePtr<ProfileChunkedBuffer>`.
It can just be empty, because it won't actually be used anyway.
Added non-`MOZ_GECKO_PROFILER` tests around this.
Differential Revision: https://phabricator.services.mozilla.com/D89351
Port handling of DecoderDoctor notifications for fission. This is done by moving
the observing of 'decoder-doctor-notification' from browser-media.js to fission
friendly actors.
As browser-media.js had only code related to this patch left it is removed along
with references to it. Same with ContentObservers.js.
Differential Revision: https://phabricator.services.mozilla.com/D88322
This patch ports existing ProfilerMarkerPayload types to draft struct definitions that may be used with the new markers API.
This is just a starting point, they may be changed later on as needed, see meta-bug 1661394.
Differential Revision: https://phabricator.services.mozilla.com/D87259
This is the main public marker API:
- `AddMarkerToBuffer` can be used to store a marker to any buffer. This could be useful to code that wants to store markers outside of the default profiler buffers.
- `baseprofiler::AddMarker`/`profiler_add_marker` store a marker in the appropriate profiler buffer.
- BASE_PROFILER_MARKER and PROFILER_MARKER do the same, but are also defined (and empty) when MOZ_GECKO_PROFILER is not #defined.
All these take a name, marker options, a marker type, and the type's expected arguments if any (as expected by the `StreamJSONMarkerData` function).
Extra helpers for the most common types:
- BASE_PROFILER_MARKER_UNTYPED and PROFILER_MARKER_UNTYPED store a marker with no data payload.
- BASE_PROFILER_MARKER_TEXT and PROFILER_MARKER_TEXT store a text marker. `baseprofiler::markers::Text` is an example of how new marker types can be defined.
Differential Revision: https://phabricator.services.mozilla.com/D87257
`DeserializeAfterKindAndStream()` is the main function that extracts all the marker data (past the already-read entry kind), and streams it to JSON using the user-provided `Stream(JSONWriter&, ...)` function in the appropriate marker type definition.
It currently requires two external functions to stream the name and the optional backtrace, because these are different between the two profilers. This may change in the future.
(Deserialization is implemented before serialization, because the `Deserialize()` function is needed during serialization to get a marker type tag.)
Differential Revision: https://phabricator.services.mozilla.com/D87254
`NoPayload` will be mostly used internally when adding markers without payload data.
It has an empty specialization of the MarkerTypeHelper (mainly to catch misuses), and the add-marker code will need to have different compile-time paths to handle it.
Differential Revision: https://phabricator.services.mozilla.com/D87252
`profiler_capture_backtrace(ProfileChunkedBuffer&)` renamed to `profiler_capture_backtrace_into(ProfileChunkedBuffer&)` (notice "_into"), which is clearer.
New function `profiler_capture_backtrace()` creates a buffer, uses `profiler_capture_backtrace_into()`, and returns a `UniquePtr<ProfileChunkedBuffer>`, which can later be given to `MarkerStack::TakeBacktrace`.
`profiler_get_backtrace()` (returning a `UniqueProfilerBacktrace`) now uses `profiler_capture_backtrace()`.
This patch reduces most duplicate code between these functions.
Differential Revision: https://phabricator.services.mozilla.com/D88280
This moves the existing MarkerTiming class introduced in bug 1640969 to the BaseProfilerMarkersPrerequesites.h header, and can be used as a marker option.
Some minor clarifying changes:
- `Instant()` is split into two functions: `InstantNow()` and `InstantAt(TimeStamp)`.
- `Interval(TimeStamp, TimeStamp)` must be given both start and end, otherwise `IntervalUntilNowFrom(TimeStamp)` takes the start only and ends "now".
Also the default construction is now reserved for internal marker usage, the private member function `IsUnspecified()` will be used by the add-marker code will replace it with `InstantNow()`.
The serialization contains the phase, and only one or two timestamps as needed, to save space for non-interval timings.
Differential Revision: https://phabricator.services.mozilla.com/D87245
The upcoming profiler-specific add-marker function will need to know which `ProfileChunkedBuffer` to serialize to, `profiler_get_core_buffer()` give access to the profiler's buffer, and `CachedCoreBuffer()` keeps it stored in a function-static object.
Differential Revision: https://phabricator.services.mozilla.com/D87256
These string views are similar to `std::string_view`, but they are optimized to be serialized in the profiler buffer, and later deserialized and streamed to JSON.
They accept literal strings, and keep them as unowned raw pointers and sizes.
They also accept any substring reference, assuming that they will only be used as parameters during function calls, and therefore the dependent string will live during that call where these `StringView`'s are used.
Internally, they also allow optional string ownership, which is only used during deserialization and streaming.
This is hidden, so that users are not tempted to use potentially expensive string allocations during profiling; it's only used *after* profiling, so it's less of an impact to allocate strings then. (But it could still be optimized later on, as part of bug 1577656.)
Differential Revision: https://phabricator.services.mozilla.com/D87242
This patch introduces all new files that contain the new markers C++ API and implementation.
They are mostly empty at this time, only including each other as eventually needed, and with `#ifdef MOZ_GECKO_PROFILER` guards.
Rough inclusion diagram: (header <-- includer)
BaseProfilerMarkerPrerequesites.h <-- ProfilerMarkerPrerequesites.h (Useful types: Input string view, marker options)
^ ^
BaseProfilerMarkerDetail.h <-- ProfilerMarkerDetail.h (Implementation details)
^ ^
BaseProfilerMarkers.h <-- ProfilerMarkers.h (Main API)
^ ^--------- ^ ^---------
BaseProfilerMarkerTypes.h | <-- ProfilerMarkerTypes.h | (Common marker types)
^ BaseProfiler.h <-- GeckoProfiler.h (Existing main profiler headers)
Differential Revision: https://phabricator.services.mozilla.com/D87241
This patch adds the idle and idle-bg tests to raptor-browsertime. To make these work, a new browsertime script was built which handles waiting on a given page url, as well as backgrounding the application.
Differential Revision: https://phabricator.services.mozilla.com/D88491
This patch ports existing ProfilerMarkerPayload types to draft struct definitions that may be used with the new markers API.
This is just a starting point, they may be changed later on as needed, see meta-bug 1661394.
Differential Revision: https://phabricator.services.mozilla.com/D87259
This is the main public marker API:
- `AddMarkerToBuffer` can be used to store a marker to any buffer. This could be useful to code that wants to store markers outside of the default profiler buffers.
- `baseprofiler::AddMarker`/`profiler_add_marker` store a marker in the appropriate profiler buffer.
- BASE_PROFILER_MARKER and PROFILER_MARKER do the same, but are also defined (and empty) when MOZ_GECKO_PROFILER is not #defined.
All these take a name, marker options, a marker type, and the type's expected arguments if any (as expected by the `StreamJSONMarkerData` function).
Extra helpers for the most common types:
- BASE_PROFILER_MARKER_UNTYPED and PROFILER_MARKER_UNTYPED store a marker with no data payload.
- BASE_PROFILER_MARKER_TEXT and PROFILER_MARKER_TEXT store a text marker. `baseprofiler::markers::Text` is an example of how new marker types can be defined.
Differential Revision: https://phabricator.services.mozilla.com/D87257
`DeserializeAfterKindAndStream()` is the main function that extracts all the marker data (past the already-read entry kind), and streams it to JSON using the user-provided `Stream(JSONWriter&, ...)` function in the appropriate marker type definition.
It currently requires two external functions to stream the name and the optional backtrace, because these are different between the two profilers. This may change in the future.
(Deserialization is implemented before serialization, because the `Deserialize()` function is needed during serialization to get a marker type tag.)
Differential Revision: https://phabricator.services.mozilla.com/D87254
`NoPayload` will be mostly used internally when adding markers without payload data.
It has an empty specialization of the MarkerTypeHelper (mainly to catch misuses), and the add-marker code will need to have different compile-time paths to handle it.
Differential Revision: https://phabricator.services.mozilla.com/D87252
`profiler_capture_backtrace(ProfileChunkedBuffer&)` renamed to `profiler_capture_backtrace_into(ProfileChunkedBuffer&)` (notice "_into"), which is clearer.
New function `profiler_capture_backtrace()` creates a buffer, uses `profiler_capture_backtrace_into()`, and returns a `UniquePtr<ProfileChunkedBuffer>`, which can later be given to `MarkerStack::TakeBacktrace`.
`profiler_get_backtrace()` (returning a `UniqueProfilerBacktrace`) now uses `profiler_capture_backtrace()`.
This patch reduces most duplicate code between these functions.
Differential Revision: https://phabricator.services.mozilla.com/D88280
This moves the existing MarkerTiming class introduced in bug 1640969 to the BaseProfilerMarkersPrerequesites.h header, and can be used as a marker option.
Some minor clarifying changes:
- `Instant()` is split into two functions: `InstantNow()` and `InstantAt(TimeStamp)`.
- `Interval(TimeStamp, TimeStamp)` must be given both start and end, otherwise `IntervalUntilNowFrom(TimeStamp)` takes the start only and ends "now".
Also the default construction is now reserved for internal marker usage, the private member function `IsUnspecified()` will be used by the add-marker code will replace it with `InstantNow()`.
The serialization contains the phase, and only one or two timestamps as needed, to save space for non-interval timings.
Differential Revision: https://phabricator.services.mozilla.com/D87245
The upcoming profiler-specific add-marker function will need to know which `ProfileChunkedBuffer` to serialize to, `profiler_get_core_buffer()` give access to the profiler's buffer, and `CachedCoreBuffer()` keeps it stored in a function-static object.
Differential Revision: https://phabricator.services.mozilla.com/D87256
These string views are similar to `std::string_view`, but they are optimized to be serialized in the profiler buffer, and later deserialized and streamed to JSON.
They accept literal strings, and keep them as unowned raw pointers and sizes.
They also accept any substring reference, assuming that they will only be used as parameters during function calls, and therefore the dependent string will live during that call where these `StringView`'s are used.
Internally, they also allow optional string ownership, which is only used during deserialization and streaming.
This is hidden, so that users are not tempted to use potentially expensive string allocations during profiling; it's only used *after* profiling, so it's less of an impact to allocate strings then. (But it could still be optimized later on, as part of bug 1577656.)
Differential Revision: https://phabricator.services.mozilla.com/D87242
This patch introduces all new files that contain the new markers C++ API and implementation.
They are mostly empty at this time, only including each other as eventually needed, and with `#ifdef MOZ_GECKO_PROFILER` guards.
Rough inclusion diagram: (header <-- includer)
BaseProfilerMarkerPrerequesites.h <-- ProfilerMarkerPrerequesites.h (Useful types: Input string view, marker options)
^ ^
BaseProfilerMarkerDetail.h <-- ProfilerMarkerDetail.h (Implementation details)
^ ^
BaseProfilerMarkers.h <-- ProfilerMarkers.h (Main API)
^ ^--------- ^ ^---------
BaseProfilerMarkerTypes.h | <-- ProfilerMarkerTypes.h | (Common marker types)
^ BaseProfiler.h <-- GeckoProfiler.h (Existing main profiler headers)
Differential Revision: https://phabricator.services.mozilla.com/D87241
This patch ports existing ProfilerMarkerPayload types to draft struct definitions that may be used with the new markers API.
This is just a starting point, they may be changed later on as needed, see meta-bug 1661394.
Differential Revision: https://phabricator.services.mozilla.com/D87259
This is the main public marker API:
- `AddMarkerToBuffer` can be used to store a marker to any buffer. This could be useful to code that wants to store markers outside of the default profiler buffers.
- `baseprofiler::AddMarker`/`profiler_add_marker` store a marker in the appropriate profiler buffer.
- BASE_PROFILER_MARKER and PROFILER_MARKER do the same, but are also defined (and empty) when MOZ_GECKO_PROFILER is not #defined.
All these take a name, marker options, a marker type, and the type's expected arguments if any (as expected by the `StreamJSONMarkerData` function).
Extra helpers for the most common types:
- BASE_PROFILER_MARKER_UNTYPED and PROFILER_MARKER_UNTYPED store a marker with no data payload.
- BASE_PROFILER_MARKER_TEXT and PROFILER_MARKER_TEXT store a text marker. `baseprofiler::markers::Text` is an example of how new marker types can be defined.
Differential Revision: https://phabricator.services.mozilla.com/D87257
`DeserializeAfterKindAndStream()` is the main function that extracts all the marker data (past the already-read entry kind), and streams it to JSON using the user-provided `Stream(JSONWriter&, ...)` function in the appropriate marker type definition.
It currently requires two external functions to stream the name and the optional backtrace, because these are different between the two profilers. This may change in the future.
(Deserialization is implemented before serialization, because the `Deserialize()` function is needed during serialization to get a marker type tag.)
Differential Revision: https://phabricator.services.mozilla.com/D87254
`NoPayload` will be mostly used internally when adding markers without payload data.
It has an empty specialization of the MarkerTypeHelper (mainly to catch misuses), and the add-marker code will need to have different compile-time paths to handle it.
Differential Revision: https://phabricator.services.mozilla.com/D87252
`profiler_capture_backtrace(ProfileChunkedBuffer&)` renamed to `profiler_capture_backtrace_into(ProfileChunkedBuffer&)` (notice "_into"), which is clearer.
New function `profiler_capture_backtrace()` creates a buffer, uses `profiler_capture_backtrace_into()`, and returns a `UniquePtr<ProfileChunkedBuffer>`, which can later be given to `MarkerStack::TakeBacktrace`.
`profiler_get_backtrace()` (returning a `UniqueProfilerBacktrace`) now uses `profiler_capture_backtrace()`.
This patch reduces most duplicate code between these functions.
Differential Revision: https://phabricator.services.mozilla.com/D88280
This moves the existing MarkerTiming class introduced in bug 1640969 to the BaseProfilerMarkersPrerequesites.h header, and can be used as a marker option.
Some minor clarifying changes:
- `Instant()` is split into two functions: `InstantNow()` and `InstantAt(TimeStamp)`.
- `Interval(TimeStamp, TimeStamp)` must be given both start and end, otherwise `IntervalUntilNowFrom(TimeStamp)` takes the start only and ends "now".
Also the default construction is now reserved for internal marker usage, the private member function `IsUnspecified()` will be used by the add-marker code will replace it with `InstantNow()`.
The serialization contains the phase, and only one or two timestamps as needed, to save space for non-interval timings.
Differential Revision: https://phabricator.services.mozilla.com/D87245
The upcoming profiler-specific add-marker function will need to know which `ProfileChunkedBuffer` to serialize to, `profiler_get_core_buffer()` give access to the profiler's buffer, and `CachedCoreBuffer()` keeps it stored in a function-static object.
Differential Revision: https://phabricator.services.mozilla.com/D87256
These string views are similar to `std::string_view`, but they are optimized to be serialized in the profiler buffer, and later deserialized and streamed to JSON.
They accept literal strings, and keep them as unowned raw pointers and sizes.
They also accept any substring reference, assuming that they will only be used as parameters during function calls, and therefore the dependent string will live during that call where these `StringView`'s are used.
Internally, they also allow optional string ownership, which is only used during deserialization and streaming.
This is hidden, so that users are not tempted to use potentially expensive string allocations during profiling; it's only used *after* profiling, so it's less of an impact to allocate strings then. (But it could still be optimized later on, as part of bug 1577656.)
Differential Revision: https://phabricator.services.mozilla.com/D87242
This patch introduces all new files that contain the new markers C++ API and implementation.
They are mostly empty at this time, only including each other as eventually needed, and with `#ifdef MOZ_GECKO_PROFILER` guards.
Rough inclusion diagram: (header <-- includer)
BaseProfilerMarkerPrerequesites.h <-- ProfilerMarkerPrerequesites.h (Useful types: Input string view, marker options)
^ ^
BaseProfilerMarkerDetail.h <-- ProfilerMarkerDetail.h (Implementation details)
^ ^
BaseProfilerMarkers.h <-- ProfilerMarkers.h (Main API)
^ ^--------- ^ ^---------
BaseProfilerMarkerTypes.h | <-- ProfilerMarkerTypes.h | (Common marker types)
^ BaseProfiler.h <-- GeckoProfiler.h (Existing main profiler headers)
Differential Revision: https://phabricator.services.mozilla.com/D87241
This patch ports existing ProfilerMarkerPayload types to draft struct definitions that may be used with the new markers API.
This is just a starting point, they may be changed later on as needed, see meta-bug 1661394.
Differential Revision: https://phabricator.services.mozilla.com/D87259
This is the main public marker API:
- `AddMarkerToBuffer` can be used to store a marker to any buffer. This could be useful to code that wants to store markers outside of the default profiler buffers.
- `baseprofiler::AddMarker`/`profiler_add_marker` store a marker in the appropriate profiler buffer.
- BASE_PROFILER_MARKER and PROFILER_MARKER do the same, but are also defined (and empty) when MOZ_GECKO_PROFILER is not #defined.
All these take a name, marker options, a marker type, and the type's expected arguments if any (as expected by the `StreamJSONMarkerData` function).
Extra helpers for the most common types:
- BASE_PROFILER_MARKER_UNTYPED and PROFILER_MARKER_UNTYPED store a marker with no data payload.
- BASE_PROFILER_MARKER_TEXT and PROFILER_MARKER_TEXT store a text marker. `baseprofiler::markers::Text` is an example of how new marker types can be defined.
Differential Revision: https://phabricator.services.mozilla.com/D87257
`DeserializeAfterKindAndStream()` is the main function that extracts all the marker data (past the already-read entry kind), and streams it to JSON using the user-provided `Stream(JSONWriter&, ...)` function in the appropriate marker type definition.
It currently requires two external functions to stream the name and the optional backtrace, because these are different between the two profilers. This may change in the future.
(Deserialization is implemented before serialization, because the `Deserialize()` function is needed during serialization to get a marker type tag.)
Differential Revision: https://phabricator.services.mozilla.com/D87254
`NoPayload` will be mostly used internally when adding markers without payload data.
It has an empty specialization of the MarkerTypeHelper (mainly to catch misuses), and the add-marker code will need to have different compile-time paths to handle it.
Differential Revision: https://phabricator.services.mozilla.com/D87252
`profiler_capture_backtrace(ProfileChunkedBuffer&)` renamed to `profiler_capture_backtrace_into(ProfileChunkedBuffer&)` (notice "_into"), which is clearer.
New function `profiler_capture_backtrace()` creates a buffer, uses `profiler_capture_backtrace_into()`, and returns a `UniquePtr<ProfileChunkedBuffer>`, which can later be given to `MarkerStack::TakeBacktrace`.
`profiler_get_backtrace()` (returning a `UniqueProfilerBacktrace`) now uses `profiler_capture_backtrace()`.
This patch reduces most duplicate code between these functions.
Differential Revision: https://phabricator.services.mozilla.com/D88280
This moves the existing MarkerTiming class introduced in bug 1640969 to the BaseProfilerMarkersPrerequesites.h header, and can be used as a marker option.
Some minor clarifying changes:
- `Instant()` is split into two functions: `InstantNow()` and `InstantAt(TimeStamp)`.
- `Interval(TimeStamp, TimeStamp)` must be given both start and end, otherwise `IntervalUntilNowFrom(TimeStamp)` takes the start only and ends "now".
Also the default construction is now reserved for internal marker usage, the private member function `IsUnspecified()` will be used by the add-marker code will replace it with `InstantNow()`.
The serialization contains the phase, and only one or two timestamps as needed, to save space for non-interval timings.
Differential Revision: https://phabricator.services.mozilla.com/D87245
The upcoming profiler-specific add-marker function will need to know which `ProfileChunkedBuffer` to serialize to, `profiler_get_core_buffer()` give access to the profiler's buffer, and `CachedCoreBuffer()` keeps it stored in a function-static object.
Differential Revision: https://phabricator.services.mozilla.com/D87256
These string views are similar to `std::string_view`, but they are optimized to be serialized in the profiler buffer, and later deserialized and streamed to JSON.
They accept literal strings, and keep them as unowned raw pointers and sizes.
They also accept any substring reference, assuming that they will only be used as parameters during function calls, and therefore the dependent string will live during that call where these `StringView`'s are used.
Internally, they also allow optional string ownership, which is only used during deserialization and streaming.
This is hidden, so that users are not tempted to use potentially expensive string allocations during profiling; it's only used *after* profiling, so it's less of an impact to allocate strings then. (But it could still be optimized later on, as part of bug 1577656.)
Differential Revision: https://phabricator.services.mozilla.com/D87242
This patch introduces all new files that contain the new markers C++ API and implementation.
They are mostly empty at this time, only including each other as eventually needed, and with `#ifdef MOZ_GECKO_PROFILER` guards.
Rough inclusion diagram: (header <-- includer)
BaseProfilerMarkerPrerequesites.h <-- ProfilerMarkerPrerequesites.h (Useful types: Input string view, marker options)
^ ^
BaseProfilerMarkerDetail.h <-- ProfilerMarkerDetail.h (Implementation details)
^ ^
BaseProfilerMarkers.h <-- ProfilerMarkers.h (Main API)
^ ^--------- ^ ^---------
BaseProfilerMarkerTypes.h | <-- ProfilerMarkerTypes.h | (Common marker types)
^ BaseProfiler.h <-- GeckoProfiler.h (Existing main profiler headers)
Differential Revision: https://phabricator.services.mozilla.com/D87241
This test begins to fail due to a `pip` installation issue after bug 1660351. It's already disabled on macOS (also due to what the comment says is a `pip` installation issue), so it's very possible the custom `requirements.txt` that this test installs is deficient in some way that surfaces in certain `virtualenv`s under certain circumstances. I can't diagnose the failure, but what I have seen is that bug 1659539 will fix that problem entirely with no extra intervention required, so we should be able to re-enable this for Linux (and maybe macOS as well?) when that patch lands.
Differential Revision: https://phabricator.services.mozilla.com/D88653
This change makes eslint aware of the global IOUtils interface, such that it
may be used without having to access it from the window or otherwise declaring
the global in a special comment on a file-by-file basis.
Differential Revision: https://phabricator.services.mozilla.com/D87612
In most calls to `SpliceableJSONWriter::Splice(const char*)`:
- The data comes from a `ChunkedJSONWriteFunc` and is copied to a new buffer, which is then copied again through `Write()`. Instead we can copy the data directly from the `ChunkedJSONWriteFunc`; and this is a nice complement to `TakeAndSplice()` below.
- Or the length is already known, so we can pass it to a new `Splice(const char*, size_t)`, which forwards it to `Write(const char*, size_t)`, saving one `strlen` call.
Differential Revision: https://phabricator.services.mozilla.com/D87703
`SpliceableChunkedJSONWriter::ChunkedWriteFunc` returns a `ChunkedJSONWriteFunc*`, which is never null and is either used to:
1. Copy data.
2. Or take ownership of the chunks.
In the first case, `ChunkedWriteFunc()` now returns a `const ChunkedJSONWriteFunc&` (notice "const &"), so only const members may be used to copy the data.
In the second case, a new function `TakeChunkedWriteFunc()` returns `ChunkedJSONWriteFunc&&` (notice "&&"), so it's clear that its chunks can be taken away. Some `DEBUG` assertions help ensure that it's not used anymore after that.
`TakeAndSplice()` now takes a `ChunkedJSONWriteFunc&&`.
All callers have been updated to the more appropriate functions.
Differential Revision: https://phabricator.services.mozilla.com/D87702
This change makes eslint aware of the global IOUtils interface, such that it
may be used without having to access it from the window or otherwise declaring
the global in a special comment on a file-by-file basis.
Differential Revision: https://phabricator.services.mozilla.com/D87612
Currently the `jest/no-standalone-expect` rule needs to be disabled for `jest-in-case` cases blocks as it is not compatible.
Differential Revision: https://phabricator.services.mozilla.com/D87184
To ensure correct usage of TLSs in the profiler, they are now better encapsulated so that:
- init() is called once and its result is cached. (TLSREGISTEREDThread::Init() doesn't need proof of the PSLock, because it's using thread-safe function-static initializers.)
- get() and set() always init() as needed, or in some particular cases strongly assert that init() was successful.
Also, a null-check was missing in profiler_init_threadmanager().
Depends on D87588
Differential Revision: https://phabricator.services.mozilla.com/D87589
Assertions are also clarified with messages, to better distinguish the same tests in different locations.
Assertions should now cover all cases:
- NEW: After registering a thread in the profiler with `CorePS::AppendRegisteredThread`, the TLS should be set to that thread.
- NEW: If `profiler_register_thread` is called again, the TLS should still be set to that thread.
- When `profiler_unregister_thread` is first called, the TLS should still be set to that thread (that's the assertion currently trigering this bug 1657174),
- NEW: When `profiler_unregister_thread` is first called and after we remove the thread with `CorePS::RemoveRegisteredThread`, the TLS should now be null.
- If `profiler_unregister_thread` is called again (or with a never-registered thread), the TLS should be null.
This is a further exploratory patch for bug 1657174, this will not prevent crashes, but hopefully it should give a bit more information, at least a smaller range in which the possible presumed registration/TLS race happens.
Differential Revision: https://phabricator.services.mozilla.com/D87588
There are zero uses of this `mach` command over the past 90 days according to our telemetry. There are no external references to `mach python-safety` in-tree, and indeed if you track the history of the originating bug 1468394, it appears that once the `mach` command was created, none of the follow-up work that was discussed (i.e. running this in CI and triaging failures to appropriate owners) was done over the following 2 years.
If this ever does appear to be useful in the future, we can just resurrect this code from source control.
Differential Revision: https://phabricator.services.mozilla.com/D87351
Before this patch, we were always waiting for libxul to load because we were
starting the JVM from libxul. But we needed to start this a lot earlier. Also
thinking that JVM profiler can actually run without the gecko side, we can
start this a lot earlier than we currently start. We need to check the
environmnet variables to be able to start the profiler. It looks like the best
place to do it is inside the GeckoThread.run method.
We have also a similar code for Java debugger, with maybeWaitForJavaDebugger
name. This fucntion does similar things in terms of enviromnent variable
handling.
Differential Revision: https://phabricator.services.mozilla.com/D87069
The stack sampling can be abstracted to only use a reference to a `ProfileBuffer`, and the existing `locked_profiler_get_backtrace` can provide its stack-based `ProfileBuffer` that points at a heap-based `ProfileChunkedBuffer` (the one that will be stored in the returned `ProfilerBacktrace`).
And we can now add a public `profiler_capture_backtrace` that only takes a reference to a `ProfileChunkedBuffer`, and fills it with a backtrace.
This will be used by the new marker API, to optionally capture a backtrace in stack-based buffers at the user's request.
Differential Revision: https://phabricator.services.mozilla.com/D86514
A heap-allocate ProfileBuffer is not really needed, so it's more efficient to have one on the stack during capture, and we don't need to keep it in the `ProfilerBacktrace`.
Differential Revision: https://phabricator.services.mozilla.com/D86513
Instead of always taking ownership of both heap-allocated `ProfileBuffer` and `ProfileChunkedBuffer`, `ProfilerBacktrace` can now accept:
- Unique pointers to both or either, similar to what it was before, so a ProfilerBacktrace can be kept for later use.
- Non-owning pointers to both or either, to allow callers to use stack-based buffer(s); null pointers are allowed for totally empty backtraces.
Only the `ProfileChunkedBuffer` contains the actual data, we can create a `ProfileBuffer` on the spot if not provided.
Differential Revision: https://phabricator.services.mozilla.com/D86512
Instead of keeping a pointer to a null-terminated string, it's simpler to keep a proper `std::string`, and it helps to keep the length ready for streaming.
Differential Revision: https://phabricator.services.mozilla.com/D86511
Let `ProfilerBuffer` expose its underlying `ProfileChunkedBuffer`, this will be useful when `ProfilerBacktrace` will only be given a `ProfileBuffer`, and to perform some safety checks.
As a bonus from this change, `StoreMarker()` can be made non-generic -- It was relying on both `ProfileBuffer` and `ProfileChunkedBuffer` to have the same function `PutObjects()`. Consequently, we don't need `ProfileBuffer::PutObjects()` anymore, this removes this clunky pass-through (but useful and the best solution at the time).
Differential Revision: https://phabricator.services.mozilla.com/D86510
Until now the `ProfileBuffer` would erase its attached `ProfileChunkedBuffer` upon destruction.
However:
- The main `ProfileChunkedBuffer` is erased anyway in the `ActivePS`,
- Other `ProfileChunkedBuffer`s are short-lived and don't really need to be erased.
- The upcoming changes to `ProfilerBacktrace` and its users means that we will only keep the `ProfileChunkedBuffer` as backtrace storage, a `ProfileBuffer` will only be needed during capture and then when streaming to JSON; so we don't want the `ProfileChunkedBuffer` to be erased when detached from its capturing `ProfileBuffer`.
- Also, the erasing was done by `ResetChunkManager()` in `~ProfileBuffer()`, which was asymetric with what the constructor does (nothing!). So it's better to leave whoever did the `SetChunkManager()` to deal with the corresponding `ResetChunkManager()` (in the main case `ActivePS`, otherwise short-lived buffers being destroyed at the end of their scope).
Both `ProfileBuffer` destructors were only doing this operation, so we can just remove them completely.
Differential Revision: https://phabricator.services.mozilla.com/D86509
Backtraces and other marker data will be stored directly into a ProfileChunkedBuffer from public code in both profilers, so we will need to have the entry "kinds" available outside of the profiler directories.
This also helps with de-duplicating, since the kinds will now be in one spot and shared by both profilers.
Differential Revision: https://phabricator.services.mozilla.com/D86508
While working on this bug, I found it distracting to reason about how strings (of different char types) are stored in the profile buffer.
So instead of storing the size in bytes, I think it's better to store the string length in number of characters, in particular it matches lengths as handled by string types.
Differential Revision: https://phabricator.services.mozilla.com/D86507
Gabriele Svelto
Barret Rennie
Tom Ritter
Greg Tatum
Gregory Mierzwinski
alwu
Dorel Luca
Gerald Squelart
Agi Sferro
Kartikaya Gupta
Ricky Stewart
Razvan Maries
Simon Giesecke
Sean Feng
Johann Hofmann
Butkovits Atila
Andrew Halberstadt
Bogdan Tara
Myeongjun Go
Ben Hearsum
Florian Quèze
Mike Conley
Narcis Beleuzu
Valentin Gosu
Sylvestre Ledru
Mike Hommey
Gijs Kruitbosch
Julian Descottes
Sebastian Hengst
Philipp Fischbeck
Kershaw Chang
James Graham
Nika Layzell
Christian Holler
Mats Palmgren
Robert Helmer
Mitchell Hentges
Andi-Bogdan Postelnicu
Nico Grunbaum
Michael Froman
Markus Stange
Tim Huang
Alex Chronopoulos
Shane Caraveo
Julian Seward
Alexis Beingessner
Tarek Ziadé
championshuttler
Andrew Halberstadt
Bryce Seager van Dyk
Cristina Coroiu
Brindusan Cristian
Mihai Alexandru Michis
Nick Thomas
Emil Farisan
Keefer Rourke
Bruno Bartolomasi
DesWurstes
Duncan Dean
Sonia Singla
Nazım Can Altınova