1149897 - PerformanceStatsService data is not monotonic

Status: RESOLVED FIXED

(Toolkit :: Performance Monitoring, defect)

Description

[David Teller [:Yoric] - still alive but not very active]

After switching from 16ms resolution to 1ms resolution, it appears that the data provided by the PerformanceStatsService is not monotonic. We should work around that limitation.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Attachment: monotonic.diff

This patch implements a lightweight fix to force the times used by the stopwatch monitoring to be monotonic. Also, it reactivates a subtest deactivated in bug 674779, minus a subsubtest whose fix will be more sophisticated and will need bug 1150045.

Incidentally, I suspect that the failures in the later subsubtest is due to a previous test using Jetpack to build an add-on but not cleaning up properly after itself (reading list? loop?)

Comment 2

[Jan de Mooij [:jandem]]

Comment on attachment 8586822 [details] [diff] [review]
monotonic.diff

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

::: js/src/vm/Interpreter.cpp
@@ +567,5 @@
>          kernelTimeInt.LowPart = kernelFileTime.dwLowDateTime;
>          kernelTimeInt.HighPart = kernelFileTime.dwHighDateTime;
> +        // Convert 100 ns to 1 us, make sure that the result is monotonic
> +        *systemTime = runtime_->
> +            stopwatch.systemTimeFix.monotonize(kernelTimeInt.QuadPart / 10);

This fits on one line (99 chars)

@@ +573,5 @@
>          userTimeInt.LowPart = userFileTime.dwLowDateTime;
>          userTimeInt.HighPart = userFileTime.dwHighDateTime;
> +        // Convert 100 ns to 1 us, make sure that the result is monotonic
> +        *userTime = runtime_->
> +            stopwatch.userTimeFix.monotonize(userTimeInt.QuadPart / 10);

And here.

Comment 3

[David Teller [:Yoric] - still alive but not very active]

Attachment: Making the clocks monotonic

Applied feedback.

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/39d74e5c3df5

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back this out for frequent bc3 failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/dfee21ce5b78

https://treeherder.mozilla.org/logviewer.html#?job_id=8433454&repo=mozilla-inbound

Comment 6

[David Teller [:Yoric] - still alive but not very active]

I believe that what you encountered is actually bug 1150259, which is anterior. I'll probably want to deactivate the test under Windows XP.

Comment 7

[David Teller [:Yoric] - still alive but not very active]

So, Wes, how do you want to handle this? I have just put up for review a wallpaper patch for bug 1150259, btw.

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Disabling your way to victory is more than acceptable to me. :)

Comment 9

[David Teller [:Yoric] - still alive but not very active]

In that case, let's land this bug, and let's handle bug 1150259 in bug 1150259.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/b342115be45c

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/b342115be45c