1157870 - Performance Groups should have a unique id

Status: RESOLVED FIXED

(Toolkit :: Performance Monitoring, defect)

Description

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

Right now, we have bugs such as bug 1151240 because we don't have a reliable way of checking whether two instances of PerformanceData are attached to the same piece of code. We need to fix this.

Comment 1

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

Attachment: MozReview Request: Bug 1157870 - Performance Groups should have a unique ID (low-level);r=jandem

Bug 1157870 - Performance Groups should have a unique ID (low-level);r=jandem

Comment 2

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

Attachment: MozReview Request: Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 8614816 [details]
MozReview Request: Bug 1157870 - Performance Groups should have a unique ID (low-level);r=jandem

https://reviewboard.mozilla.org/r/9829/#review8961

r=me with nits addressed.

::: js/src/vm/Runtime.cpp:964
(Diff revision 1)
> +    : stopwatch_(nullptr)
> +    , iteration_(0)
> +    , uid(JS_GetRuntime(cx)->stopwatch.uniqueId())
> +    , key_(key)
> +    , refCount_(0)

Nit: indent these 5 lines with 2 instead of 4 spaces.

::: js/src/vm/Runtime.h:1617
(Diff revision 1)
> +        uint64_t idCounter_;

This value should be initialized to 0 in the Stopwatch constructor.

::: js/src/vm/Runtime.cpp:966
(Diff revision 1)
> +    , uid(JS_GetRuntime(cx)->stopwatch.uniqueId())

Inside JS, you can use cx->runtime() instead of JS_GetRuntime.

Comment 4

[Dave Townsend [:mossop]]

Comment on attachment 8614817 [details]
MozReview Request: Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

https://reviewboard.mozilla.org/r/9831/#review8987

Ship It!

Comment 5

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

Comment on attachment 8614816 [details]
MozReview Request: Bug 1157870 - Performance Groups should have a unique ID (low-level);r=jandem

Bug 1157870 - Performance Groups should have a unique ID (low-level);r=jandem

Comment 6

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

Comment on attachment 8614817 [details]
MozReview Request: Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

Comment 7

[Dave Townsend [:mossop]]

Comment on attachment 8614817 [details]
MozReview Request: Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

https://reviewboard.mozilla.org/r/9831/#review9021

Ship It!

Comment 8

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

Comment on attachment 8614816 [details]
MozReview Request: Bug 1157870 - Performance Groups should have a unique ID (low-level);r=jandem

Bug 1157870 - Performance Groups should have a unique ID (low-level);r=jandem

Comment 9

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

Comment on attachment 8614817 [details]
MozReview Request: Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

Comment 10

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

Comment on attachment 8614817 [details]
MozReview Request: Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

Comment 11

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

Comment on attachment 8614817 [details]
MozReview Request: Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

Comment 12

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

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9c94ac253e12

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing.

Comment 14

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

Comment on attachment 8614816 [details]
MozReview Request: Bug 1157870 - Performance Groups should have a unique ID (low-level);r=jandem

Bug 1157870 - Performance Groups should have a unique ID (low-level);r=jandem

Comment 15

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

Comment on attachment 8614817 [details]
MozReview Request: Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

Comment 16

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

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

Comment 17

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/6a0397d889c9
https://hg.mozilla.org/integration/fx-team/rev/819d7887fdc4

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Backed out for frequent e10s browser_compartments.js failures.
https://treeherder.mozilla.org/logviewer.html#?job_id=3459607&repo=fx-team

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

Comment 19

[Dave Townsend [:mossop]]

Comment on attachment 8614817 [details]
MozReview Request: Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

https://reviewboard.mozilla.org/r/9831/#review9763

Ship It!

Comment 20

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

Comment on attachment 8614816 [details]
MozReview Request: Bug 1157870 - Performance Groups should have a unique ID (low-level);r=jandem

Bug 1157870 - Performance Groups should have a unique ID (low-level);r=jandem

Comment 21

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

Comment on attachment 8614817 [details]
MozReview Request: Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

Bug 1157870 - Performance Groups should have a unique ID (high-level);r=mossop

Comment 22

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

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=40217c68af2d

Comment 23

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

Well, Try looks good even when you repeat the right tests :)

Comment 24

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/8156f7d328a7
https://hg.mozilla.org/integration/fx-team/rev/7af2df7da870

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8156f7d328a7
https://hg.mozilla.org/mozilla-central/rev/7af2df7da870

Comment 26

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/19f89ed00b49

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/19f89ed00b49