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
Kartikaya Gupta
Sylvestre Ledru
Greg Tatum
Gijs Kruitbosch
Gerald Squelart
Bogdan Tara
Gregory Mierzwinski
Florian Quèze
Julian Descottes
Sebastian Hengst
Philipp Fischbeck
Kershaw Chang
James Graham
Nika Layzell
Christian Holler
Mats Palmgren
Robert Helmer
Mitchell Hentges
Ricky Stewart
Simon Giesecke
Andi-Bogdan Postelnicu
Nico Grunbaum
Michael Froman
Narcis Beleuzu
Markus Stange
Tim Huang
Alex Chronopoulos
Shane Caraveo
Dorel Luca
Sean Feng
Julian Seward