1193821 - Add thread and process CPU time for performance object

Status: NEW

(Core :: DOM: Core & HTML, defect)

Description

[Eugen Sawin [:esawin]]

We're interested in measuring CPU time usage in addition to wall clock time (performance.now) in chrome to investigate performance issues (on Android).

For this, we could expose a CPU time API through the performance object to make it accessible using JS.

Comment 1

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1193821-Add-process-and-thread-clock-API-to-perf.patch

This patch makes thread and process time accessible through a chrome-only API in the performance object on POSIX platforms.

Comment 2

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1193821-Add-process-and-thread-clock-API-to-perf.patch

Removed the rounding and time offsetting.

Comment 3

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1193821-Add-chrome-only-process-and-thread-clock.patch

Comment 4

[Eugen Sawin [:esawin]]

Attachment: 0002-Bug-1193821-Add-POSIX-implementation-of-the-process-.patch

Comment 5

[Eugen Sawin [:esawin]]

Attachment: 0003-Bug-1193821-Add-Windows-implementation-of-the-proces.patch

Comment 6

[Eugen Sawin [:esawin]]

Attachment: 0004-Bug-1193821-Add-Darwin-implementation-for-process-an.patch

Comment 7

[Eugen Sawin [:esawin]]

Attachment: 0005-Bug-1193821-Add-CPU-time-API-sanity-check.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d602be504c5

Comment 8

[Bobby Holley (:bholley)]

Comment on attachment 8652343 [details] [diff] [review]
0001-Bug-1193821-Add-chrome-only-process-and-thread-clock.patch

Review of attachment 8652343 [details] [diff] [review]:
-----------------------------------------------------------------

The DOM pieces look fine. r+ on those, r- on the timestamp thing.

I think you should flag glandium for all of that stuff.

Comment 9

[Bobby Holley (:bholley)]

Comment on attachment 8652484 [details] [diff] [review]
0002-Bug-1193821-Add-POSIX-implementation-of-the-process-.patch

glandium for the rest of these.

Comment 10

[Bobby Holley (:bholley)]

Comment on attachment 8652343 [details] [diff] [review]
0001-Bug-1193821-Add-chrome-only-process-and-thread-clock.patch

Sorry, my review comment (the reason for the r-) got swallowed somehow. :-(

basically, I think |TimeStamp::ToDuration()| is wrong - we explicitly don't expose a method to get an absolute representation out of TimeStamp, and there are lots of comments explaining why we don't do that. We should make these numbers relative to something, the way that Performance::Now does.

Comment 11

[Mike Hommey [:glandium]]

Comment on attachment 8652484 [details] [diff] [review]
0002-Bug-1193821-Add-POSIX-implementation-of-the-process-.patch

Review of attachment 8652484 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozglue/misc/TimeStamp_posix.cpp
@@ +88,5 @@
>  static uint64_t
> +ClockThreadTimeNs()
> +{
> +  struct timespec ts;
> +  clock_gettime(CLOCK_THREAD_CPUTIME_ID, &ts);

Not all POSIX systems have CLOCK_THREAD_CPUTIME_ID and CLOCK_PROCESS_CPUTIME_ID, so you need to add some #ifdef and provide a sane fallback. Also, you should check the return value for clock_gettime in case the clock id is not supported (in which case it will return -1 and set errno to EINVAL).

Comment 12

[Mike Hommey [:glandium]]

Comment on attachment 8652485 [details] [diff] [review]
0003-Bug-1193821-Add-Windows-implementation-of-the-proces.patch

Review of attachment 8652485 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozglue/misc/TimeStamp_windows.cpp
@@ +538,5 @@
> +
> +  union {
> +    // This holds the time value in 100ns steps.
> +    FILETIME fileTime;
> +    int64_t int64;

This is a confusing name to use as a member name.

Note we have a FileTimeToMicroseconds function in ipc/chromium/src/base/time_win.cc that while it can't be used here, can be an inspiration: it uses mozilla::BitwiseCast in a one-liner, that would avoid the union completely.

@@ +550,5 @@
> +  FILETIME exitTime;
> +  FileTimeContainer kernelTime;
> +  FileTimeContainer userTime;
> +
> +  // We can't use a null timestamp for arithmetic operations.

It doesn't appear obvious why. Care to elaborate?

Comment 13

[Mike Hommey [:glandium]]

Comment on attachment 8652487 [details] [diff] [review]
0004-Bug-1193821-Add-Darwin-implementation-for-process-an.patch

Review of attachment 8652487 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozglue/misc/TimeStamp_darwin.cpp
@@ +185,5 @@
> +      != KERN_SUCCESS) {
> +    return 0;
> +  }
> +
> +  uint64_t threadTime = threadInfo.user_time.seconds * 1e9 +

I don't remember if exponential notation leads to floating point types or integer types. I'd rather err on the side of safety and just use plain integers.

@@ +230,5 @@
> +      processTime += threadInfo.user_time.seconds * 1e9 +
> +                     threadInfo.user_time.microseconds * 1e3 +
> +                     threadInfo.system_time.seconds * 1e9 +
> +                     threadInfo.system_time.microseconds * 1e3;
> +    }

You can use TASK_THREAD_TIMES_INFO to get all of them at once instead of going through them manually one by one.

Comment 14

[Mike Hommey [:glandium]]

That was for the individual implementations. Now, on a higher level, using the same type as TimeStamp::Now is not a good idea, because it won't prevent values to be mixed and wrong results to ensue. You need to have dedicated types for each.

With all that being said, I see there is JSRuntime::Stopwatch::getResources that does the same thing partially, and JSRuntime::Stopwatch::start/stop using it. Is there a reason not to use that instead, or not to base your stuff on that?

Comment 15

[Eugen Sawin [:esawin]]

(In reply to Mike Hommey [:glandium] from comment #12)
> @@ +550,5 @@
> > +  FILETIME exitTime;
> > +  FileTimeContainer kernelTime;
> > +  FileTimeContainer userTime;
> > +
> > +  // We can't use a null timestamp for arithmetic operations.
> 
> It doesn't appear obvious why. Care to elaborate?

A null TimeStamp is reserved for non-initialized timestamps, we assert against arithmetic operations on it.

(In reply to Mike Hommey [:glandium] from comment #14)
> That was for the individual implementations. Now, on a higher level, using
> the same type as TimeStamp::Now is not a good idea, because it won't prevent
> values to be mixed and wrong results to ensue. You need to have dedicated
> types for each.

I didn't view TimeStamp to be specific to realtime (Now) as most of its features aren't. Type safety comes at the cost of code duplication in this case since the dedicated types would share the arithmetic and comparison operators with TimeStamp. I don't see a clean way to share those operators without allowing for polymorphic interchange between the types.

> With all that being said, I see there is JSRuntime::Stopwatch::getResources
> that does the same thing partially, and JSRuntime::Stopwatch::start/stop
> using it. Is there a reason not to use that instead, or not to base your
> stuff on that?

We want to have explicit access to thread and process times and Stopwatch does not allow for that. Basing the timers on Stopwatch would be more confusing than it would help, since the types of the times returned there depend on the platform.

I'll address all the other comments for the next review round.

Comment 16

[Mike Hommey [:glandium]]

(In reply to Eugen Sawin [:esawin] from comment #15)
> (In reply to Mike Hommey [:glandium] from comment #12)
> > @@ +550,5 @@
> > > +  FILETIME exitTime;
> > > +  FileTimeContainer kernelTime;
> > > +  FileTimeContainer userTime;
> > > +
> > > +  // We can't use a null timestamp for arithmetic operations.
> > 
> > It doesn't appear obvious why. Care to elaborate?
> 
> A null TimeStamp is reserved for non-initialized timestamps, we assert
> against arithmetic operations on it.

I don't see such asserts:
https://dxr.mozilla.org/mozilla-central/source/mozglue/misc/TimeStamp_windows.h?offset=100#44-51
https://dxr.mozilla.org/mozilla-central/source/mozglue/misc/TimeStamp_windows.cpp#298-312
https://dxr.mozilla.org/mozilla-central/source/mozglue/misc/TimeStamp_windows.cpp#391-398

> (In reply to Mike Hommey [:glandium] from comment #14)
> > That was for the individual implementations. Now, on a higher level, using
> > the same type as TimeStamp::Now is not a good idea, because it won't prevent
> > values to be mixed and wrong results to ensue. You need to have dedicated
> > types for each.
> 
> I didn't view TimeStamp to be specific to realtime (Now) as most of its
> features aren't.

Most of its features are about time comparisons and differences. Which assumes they are all from the same time source. If we introduce new time sources, they should be explicitly incompatible.

> Type safety comes at the cost of code duplication in this
> case since the dedicated types would share the arithmetic and comparison
> operators with TimeStamp. I don't see a clean way to share those operators
> without allowing for polymorphic interchange between the types.

Inherit from a templated class?

> > With all that being said, I see there is JSRuntime::Stopwatch::getResources
> > that does the same thing partially, and JSRuntime::Stopwatch::start/stop
> > using it. Is there a reason not to use that instead, or not to base your
> > stuff on that?
> 
> We want to have explicit access to thread and process times and Stopwatch
> does not allow for that. Basing the timers on Stopwatch would be more
> confusing than it would help, since the types of the times returned there
> depend on the platform.

AIUI, the intent there is to use thread time, but fallback on process time for platforms that don't provide thread time. IOW, on all platforms we actually care the most (tier-1), it should always be thread time. If that's not the case, it should probably be fixed to be thread time. Now, if you need *both* process time and thread time, why not augment Stopwatch instead of adding new TimeStamp sources?

Comment 17

[Eugen Sawin [:esawin]]

(In reply to Mike Hommey [:glandium] from comment #16)
> I don't see such asserts:
> https://dxr.mozilla.org/mozilla-central/source/mozglue/misc/
> TimeStamp_windows.h?offset=100#44-51
> https://dxr.mozilla.org/mozilla-central/source/mozglue/misc/
> TimeStamp_windows.cpp#298-312
> https://dxr.mozilla.org/mozilla-central/source/mozglue/misc/
> TimeStamp_windows.cpp#391-398

These are the (internal) TimeValueType operators, but we would call the TimeStamp operators and hit the assert in [1].

[1] https://dxr.mozilla.org/mozilla-central/source/mozglue/misc/TimeStamp.h?offset=0#494

I've addressed all comments in the next patch round except for the dedicated types and reusing Stopwatch, which I will address in a follow-up comment/patch.

Comment 18

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1193821-Add-chrome-only-process-and-thread-clock.patch

Comment 19

[Eugen Sawin [:esawin]]

Attachment: 0002-Bug-1193821-Add-POSIX-implementation-of-the-process-.patch

Comment 20

[Eugen Sawin [:esawin]]

Attachment: 0003-Bug-1193821-Add-Windows-implementation-of-the-proces.patch

Comment 21

[Eugen Sawin [:esawin]]

Attachment: 0004-Bug-1193821-Add-Darwin-implementation-for-process-an.patch

Comment 22

[Eugen Sawin [:esawin]]

Attachment: 0006-Bug-1193821-Add-dedicated-time-stamp-types-for-CPU-t.patch

This is an early WIP patch to explore dedicated types based on template specialization (without inheritance). This will not cleanly compile as of now.

(In reply to Mike Hommey [:glandium] from comment #16)
> Most of its features are about time comparisons and differences. Which
> assumes they are all from the same time source. If we introduce new time
> sources, they should be explicitly incompatible.

There is no time API known to me that has dedicated time stamp types for the different clock types. I think the minimal benefit we would get with increased type safety stands in a bad relation to the increased code complexity and code bloat.

And there is nothing stopping the user from comparing incompatible TimeDuration values or the generic floats which are finally generated in chrome (which is the prime and possibly only use case for the new CPU time API) even with dedicated time stamp types.

> Inherit from a templated class?

Inheritance from class templates would additionally increase the class complexity, especially if we want to reuse the base operators (for no real benefit, see previous comment).

> AIUI, the intent there is to use thread time, but fallback on process time
> for platforms that don't provide thread time. IOW, on all platforms we
> actually care the most (tier-1), it should always be thread time. If that's
> not the case, it should probably be fixed to be thread time. Now, if you
> need *both* process time and thread time, why not augment Stopwatch instead
> of adding new TimeStamp sources?

The Stopwatch is solely used within the VM and is more specialized for that use case. It felt more natural to implement the CPU clocks in the same manner Now() was implemented, so I didn't consider it.

It looks like we would need to extend and modify the Stopwatch to support both, thread and process times. I'm not sure whether we can reuse much of it without interfering with its original use case, but I can look into it (preferably in a follow-up bug to split the work load).

Comment 23

[Mike Hommey [:glandium]]

Attachment: Using a templated base class

(In reply to Eugen Sawin [:esawin] from comment #22)
> And there is nothing stopping the user from comparing incompatible
> TimeDuration values or the generic floats which are finally generated in
> chrome (which is the prime and possibly only use case for the new CPU time
> API) even with dedicated time stamp types.

Whichever time source, TimeDurations all have the same unit. They can be used against each other without much problem. In fact, there might be cases where you would want to know how much wall clock is spent *not* using the CPU, and that's a perfectly valid operation to do. It's comparing time stamps from different time sources that makes no sense.

> > Inherit from a templated class?
> 
> Inheritance from class templates would additionally increase the class
> complexity, especially if we want to reuse the base operators (for no real
> benefit, see previous comment).

Since I tried it to verify your claim, here is a version of TimeStamp using a base template class. The only "bloat" required for it to work is 2 constructors, and a friend definition.

Comment 24

[Eugen Sawin [:esawin]]

Attachment: 0006-Bug-1193821-Use-dedicated-time-stamp-classes-for-CPU.patch

This is what's required to give us dedicated time stamp types for each timer based on a templated base class.

Comment 25

[Mike Hommey [:glandium]]

Comment on attachment 8656127 [details] [diff] [review]
0006-Bug-1193821-Use-dedicated-time-stamp-classes-for-CPU.patch

Review of attachment 8656127 [details] [diff] [review]:
-----------------------------------------------------------------

From a quick glance, this looks fine, but it would be easier to follow if it were retrofitted in the existing patches.

Comment 26

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1193821-Add-chrome-only-process-and-thread-clock.patch

Merged with the dedicated time stamp classes patch.

Comment 27

[Eugen Sawin [:esawin]]

Attachment: 0002-Bug-1193821-Add-POSIX-implementation-of-the-process-.patch

Comment 28

[Eugen Sawin [:esawin]]

Attachment: 0003-Bug-1193821-Add-Windows-implementation-of-the-proces.patch

Comment 29

[Eugen Sawin [:esawin]]

Attachment: 0004-Bug-1193821-Add-Darwin-implementation-for-process-an.patch

Comment 30

[Eugen Sawin [:esawin]]

Attachment: 0005-Bug-1193821-Add-CPU-time-API-sanity-check.patch

Added minimum resolution offsetting to monotonicity tests and increased number of iterations per loop, otherwise monotonicity tests would be rendered completely void.

Comment 31

[Mike Hommey [:glandium]]

Comment on attachment 8656645 [details] [diff] [review]
0002-Bug-1193821-Add-POSIX-implementation-of-the-process-.patch

Review of attachment 8656645 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozglue/misc/TimeStamp_posix.cpp
@@ +214,5 @@
> +  if (!clock_gettime(CLOCK_THREAD_CPUTIME_ID, &ts)) {
> +    return ThreadTimeStamp(TimespecToNs(ts));
> +  }
> +#endif
> +  return ThreadTimeStamp();

This will assert on debug builds when doing operations on it.

@@ +226,5 @@
> +  if (!clock_gettime(CLOCK_PROCESS_CPUTIME_ID, &ts)) {
> +    return ProcessTimeStamp(TimespecToNs(ts));
> +  }
> +#endif
> +  return ProcessTimeStamp();

Likewise

Comment 32

[Mike Hommey [:glandium]]

Comment on attachment 8656646 [details] [diff] [review]
0003-Bug-1193821-Add-Windows-implementation-of-the-proces.patch

Review of attachment 8656646 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozglue/misc/TimeStamp_windows.cpp
@@ +535,5 @@
> +double
> +FileTimeToMilliseconds(const FILETIME& ft) {
> +  // Need to BitwiseCast to fix alignment, then divide by 10000 to convert
> +  // 100-nanoseconds to milliseconds. This only works on little-endian
> +  // machines.

which all Windows machines are.

@@ +555,5 @@
> +    int64_t ticks =
> +      BaseTimeDurationPlatformUtils::TicksFromMilliseconds(timeMs);
> +    return ThreadTimeStamp(TimeStampValue(ticks, 0, false));
> +  }
> +  return ThreadTimeStamp();

Same remark as for the POSIX implementation.

@@ +574,5 @@
> +    int64_t ticks =
> +      BaseTimeDurationPlatformUtils::TicksFromMilliseconds(timeMs);
> +    return ProcessTimeStamp(TimeStampValue(ticks, 0, false));
> +  }
> +  return ProcessTimeStamp();

Likewise.

Comment 33

[Mike Hommey [:glandium]]

Comment on attachment 8656647 [details] [diff] [review]
0004-Bug-1193821-Add-Darwin-implementation-for-process-an.patch

Review of attachment 8656647 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozglue/misc/TimeStamp_darwin.cpp
@@ +193,5 @@
> +                  reinterpret_cast<thread_info_t>(&threadInfo), &numInfo)
> +      == KERN_SUCCESS) {
> +    return ThreadTimeStamp(MachInfoToNs(threadInfo));
> +  }
> +  return ThreadTimeStamp();

Same as posix and windows.

@@ +207,5 @@
> +                reinterpret_cast<task_info_t>(&threadTimesInfo), &numInfo)
> +      == KERN_SUCCESS) {
> +    return ProcessTimeStamp(MachInfoToNs(threadTimesInfo));
> +  }
> +  return ProcessTimeStamp();

Likewise

Comment 34

[Mike Hommey [:glandium]]

Maybe it would make sense for IsNull to always return false for Process and Thread TimeStamps.

Comment 35

[Mike Hommey [:glandium]]

Comment on attachment 8656651 [details] [diff] [review]
0005-Bug-1193821-Add-CPU-time-API-sanity-check.patch

Review of attachment 8656651 [details] [diff] [review]:
-----------------------------------------------------------------

This is more for bholley, but if this patch didn't change, you can also carry over the previous r+.

Comment 36

[Eugen Sawin [:esawin]]

(In reply to Mike Hommey [:glandium] from comment #34)
> Maybe it would make sense for IsNull to always return false for Process and
> Thread TimeStamps.

Would you prefer it to silently fail on debug builds if the platform does not support the requested CPU time? We could return *TimeStamp(1) in the failure case (to keep null-timestamp semantics consistent), but it might be less clear than asserting.

On release builds, both solutions would yield more or less same behaviour (null/bad time stamps and invalid durations).

(In reply to Mike Hommey [:glandium] from comment #35)
> This is more for bholley, but if this patch didn't change, you can also
> carry over the previous r+.

I'll probably just carry it over, it's a minor change (see comment 30).

Comment 37

[Eugen Sawin [:esawin]]

re: comment 36.

The options are:
1. Make IsNull return false for CPU timestamps - inconsistent null-semantics between different timestamps, allows for arithmetic operations on null timestamps.
2. Return *Timestamp(1) instead of *TimeStamp() (null timestamp) in error case - consistent null-semantics across different timestamps, allows for arithmetic operations on null timestamp.
3. Return *TimeStamp() in error case - consistent semantics, arithmetic operations would assert (on debug builds) signalizing that the timestamps are invalid.

The patch currently implements 3, which I think is a sensible solution as it retains consistency and signals an error in the case of missing CPU timer support.

Comment 38

[Eugen Sawin [:esawin]]

Option 2 does not really allow arithmetic operations on null timestamps, but we return a pseudo-null timestamp with an invalid tick value of 1 and allow operations on that.

Comment 39

[Mike Hommey [:glandium]]

3 would make some tests crash on platforms with no support, and while those platforms are not tier-1 or tier-2 and not on treeherder, we shouldn't break them knowingly.
I think I prefer 1 to 2.
Please do a try run with a patch simulating a non-supported platform.

Comment 40

[Eugen Sawin [:esawin]]

Comment on attachment 8656651 [details] [diff] [review]
0005-Bug-1193821-Add-CPU-time-API-sanity-check.patch

Carrying over r+ from bholley.

Comment 41

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1193821-Add-chrome-only-process-and-thread-clock.patch

As discussed in the previous comments, ThreadTimeStamp and ProcessTimeStamp now return false for IsNull(), which allows for arithmetic operations on null time stamps on platforms that don't support CPU timers.

Carrying over r+ from glandium.

Comment 42

[Eugen Sawin [:esawin]]

Attachment: 0003-Bug-1193821-Add-Windows-implementation-of-the-proces.patch

Removed comment on endianness.

Comment 43

[Eugen Sawin [:esawin]]

Looks like Windows is currently broken on Try (https://treeherder.mozilla.org/#/jobs?repo=try&revision=07bcb4b30a3a) which rendered my "unsupported platform" test void. I will repost once it's fixed.

Comment 44

[Eugen Sawin [:esawin]]

Comment on attachment 8656647 [details] [diff] [review]
0004-Bug-1193821-Add-Darwin-implementation-for-process-an.patch

No changes required here.

Comment 45

[Mike Hommey [:glandium]]

(In reply to Eugen Sawin [:esawin] from comment #43)
> Looks like Windows is currently broken on Try
> (https://treeherder.mozilla.org/#/jobs?repo=try&revision=07bcb4b30a3a) which
> rendered my "unsupported platform" test void. I will repost once it's fixed.

I retriggered those builds.

Comment 46

[Eugen Sawin [:esawin]]

(In reply to Mike Hommey [:glandium] from comment #45)
> (In reply to Eugen Sawin [:esawin] from comment #43)
> > Looks like Windows is currently broken on Try
> > (https://treeherder.mozilla.org/#/jobs?repo=try&revision=07bcb4b30a3a) which
> > rendered my "unsupported platform" test void. I will repost once it's fixed.
> 
> I retriggered those builds.

There are no assertions fired, which is good, but the tests pass, which I don't like. There are two reasons for this, we don't test for strict monotonicity and we expect a conservative resolution of 25ms, which allows for the time stamps to keep the same value (0 in our case) and the tests would still pass.

I don't want to make the checks stricter to prevent intermittent errors on supported platforms, but I will add an additional check against constant time stamps, to prevent unsupported platforms to pass the test.

Comment 47

[Eugen Sawin [:esawin]]

Attachment: 0005-Bug-1193821-Add-CPU-time-API-sanity-check.patch

I've changed the monotonicity tests to be strict and added a check for the absolute duration which may not be 0.

This test will fail on unsupported platforms.

Since the change to the patch is trivial, it doesn't need review, as long as we agree that the tests should fail (without assertions) on unsupported platforms.

Here are the results with simulated unsupported ThreadTime on Windows: https://treeherder.mozilla.org/#/jobs?repo=try&revision=830d72ff84fc

The regular version Try results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=270388c6ed36

Comment 48

[Mike Hommey [:glandium]]

Fair enough.

Comment 49

[Eugen Sawin [:esawin]]

Rebased Try results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d599efadac8b

Comment 50

[Eugen Sawin [:esawin]]

There are multiple issues on Try, including a failing Valgrind check on Linux and a crash in clock_gettime on the B2G ICS emulator.

https://treeherder.mozilla.org/logviewer.html#?job_id=11780953&repo=try

Comment 51

[Eugen Sawin [:esawin]]

Julian, do you know more about the Valgrind issues, is this just something we need to suppress?

Comment 52

[Eugen Sawin [:esawin]]

Attachment: 0001-Bug-1193821-Add-chrome-only-process-and-thread-clock.patch

Rebased.

Comment 53

[Eugen Sawin [:esawin]]

Attachment: 0005-Bug-1193821-Add-CPU-time-API-sanity-check.-r-bholley.patch

Rebased.

Comment 54

[Julian Seward [:jseward]]

(In reply to Eugen Sawin [:esawin] from comment #51)
I can't reproduce the Valgrind issues locally, when running with the current
dom/tests/mochitest/general/test_performance_cpu_time.html.  In any case it's
probably pretty harmless.  I suggest you ignore them.

Comment 55

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.