More exactly: Instead of trying to compensate for only the previous sleep over/undershoot, we now try to keep each sampling loop to a schedule based on the very beginning of sampling, by adding the requested interval to the scheduled sampling time.
In addition, the sleep time is always kept to a minimum, to avoid making the system busier by having one loop right after the other -- also, this very-close data may be less useful.
And in presumably very busy times, one sleep and the following sampling work may take much more time than the requested sampling interval, trying to keep to schedule is now futile (it would require trying to effectively multiply the sampling rate, which seems unlikely to succeed, and would impact Firefox even more), in which case we revert to the full sampling interval.
Differential Revision: https://phabricator.services.mozilla.com/D102253
mPostMeasurementTimeStamp records the time right after CPU measurements (point-based or interval) ended.
It is then used as the main sample timestamp, to both avoid another TimeStamp::Now() call, and to keep measurements and timestamp as close together as possible (and even closer in the next patch).
Differential Revision: https://phabricator.services.mozilla.com/D101545
This handles the conversion (from TimeStamp to number of milliseconds since process start) once and gives it to subroutines.
It will also help in a following patch where this value will be more closely tied with the CPU usage value, so we need to make sure the sample timestamp is taken at a single point and then forwarded wherever it's needed, be it a duplicate or a real sample.
While here, the nested `delta` variables in the Sampler have been disambiguated for better clarity:
- `sampleStartDeltaMs` is at the start of each sampling loop,
- `threadSampleDeltaMs` is associated to one thread being sampled during that loop.
Differential Revision: https://phabricator.services.mozilla.com/D101544
The `RunningTimes` class stores CPU measurements. It may seem overkill for only one value, but in the future more measurements will be added.
During sampling, CPU measurements are collected by platform-specific code. This patch doesn't produce anything yet, see later patches.
These are stored with the samples.
Note that for duplicated samples (when a thread is known to be "asleep"), we still need to collect new measurements, because there could potentially be some activity happening, e.g. in system calls.
Finally the measurements are output as extra "samples" values.
Units for these values may platform-specific, so they are stored in the top-level JSON "meta" object.
We don't collect running times in the Base Profiler (yet), but we still need to add the appropriate field names in the samples' "schema", as expected by profiler.firefox.com.
Differential Revision: https://phabricator.services.mozilla.com/D99413
This patch adds "CPU Utilization" ("cpu" for short) as a new feature that will control the upcoming still-experimental CPU measurements.
Differential Revision: https://phabricator.services.mozilla.com/D99054
Instead of only capturing one feature (NoStackSampling), the sampler thread now stores all features so that any feature can be quickly looked at during sampling.
Currently this is still limited to NoStackSampling, a later patch will start using another feature.
Differential Revision: https://phabricator.services.mozilla.com/D99053
The "expected maximum stack size" (currently 64KiB) value was present in multiple places.
Now it's accessible from everywhere as ProfileBufferChunkManager::scExpectedMaximumStackSize, so it's easier to modify as needed.
Differential Revision: https://phabricator.services.mozilla.com/D100222
The profiler sampler was using the "cleared block count" as indication that some data could not be stored.
But this was incorrect, because cleared blocks only account for blocks in chunks that have been destroyed or recycled, but this missed data that could not be stored due to lack of space.
In particular, `ProfileBufferChunkManagerSingle` only provides one chunk, so when it gets full, a new chunk cannot be allocated, and the data is lost; but the "cleared block count" is left unchanged (the one chunk is only released, but not destroyed nor recycled).
Now the sampler correctly looks at the "failed puts bytes", and can report precisely how many bytes could not be stored.
Differential Revision: https://phabricator.services.mozilla.com/D99978
This gtest failure was useful during early development of markers 2.0 schema, to catch all known (at the time) marker types, so they could be tested.
But now that most marker types live locally where they're used, they cannot easily be unit-tested here, so it's easy to miss some of those types. And more of them will be added in the future, potentially causing more failures here.
At the same time, these Profiler tests run along with lots of other tests, which may "naturally" produce some of those marker types, in which case the corresponding schema will be present in output profiles when running Profiler tests.
So we shouldn't make our tests fail anymore when encountering unknown marker types. They may still be verified in other tests related to the code they live in (e.g., an XPCShell test could exercise some Firefox functionality that generates markers when profiling.)
There is still a benign `printf` message, which may be useful during development, but shouldn't appear as a failure (to be fixed) in CI.
Differential Revision: https://phabricator.services.mozilla.com/D100221
This is to match the trivial change in bug 1682349.
The failure was intermittent because we only test the BHR marker schema if that marker was actually used, and there's no easy way to force it during our tests; however while running the full gtest suite, it's possible that a previous test triggered that marker and failed in the non-updated test.
Differential Revision: https://phabricator.services.mozilla.com/D99973
JSONOutputCheck used to only check the profile output for the presence of some strings.
Now it parses the output as JSON, and navigates the JSON data to check expected properties, including their types, and values as needed.
Differential Revision: https://phabricator.services.mozilla.com/D98890
JSONOutputCheck used to only check the profile output for the presence of some strings.
Now it parses the output as JSON, and navigates the JSON data to check expected properties, including their types, and values as needed.
Differential Revision: https://phabricator.services.mozilla.com/D98890
TimeStamps in markers must now be streamed through `SpliceableJSONWriter::TimeProperty(name, timestamp)`.
This is consistent with all other JSON-writing functions being in `SpliceableJSONWriter` (and base class `JSONWriter`).
Depends on D97556
Differential Revision: https://phabricator.services.mozilla.com/D97557
Florian Quèze
Gerald Squelart
Kartik Gautam
Cosmin Sabou
Csoregi Natalia
Barret Rennie
Razvan Maries
Simon Giesecke
Bogdan Tara
Christian Holler
Sylvestre Ledru
smolnar
Greg Tatum