Performance Groups should have a unique id

RESOLVED FIXED in Firefox 41

Status

()

Toolkit
Performance Monitoring
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Yoric, Assigned: Yoric)

Tracking

unspecified
mozilla41
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

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.
Blocks: 1151240
Depends on: 1149486
Created 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
Attachment #8614816 - Flags: review?(jdemooij)
Created 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
Attachment #8614817 - Flags: review?(dtownsend)
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.
Attachment #8614816 - Flags: review?(jdemooij) → review+
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!
Attachment #8614817 - Flags: review?(dtownsend) → review+
Attachment #8614816 - Flags: review+ → review?(jdemooij)
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 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
Attachment #8614817 - Flags: review+ → review?(dtownsend)
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!
Attachment #8614817 - Flags: review?(dtownsend) → review+
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
Attachment #8614816 - Flags: review?(jdemooij)
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
Attachment #8614817 - Flags: review+
Assignee: nobody → dteller
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
Attachment #8614817 - Flags: review?(dtownsend)
Attachment #8614817 - Flags: review?(dtownsend)
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
Needs rebasing.
Keywords: checkin-needed
Attachment #8614816 - Flags: review?(jdemooij)
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 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
Attachment #8614817 - Flags: review?(dtownsend)
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!
Attachment #8614817 - Flags: review?(dtownsend) → review+
Attachment #8614816 - Flags: review?(jdemooij)
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 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
Attachment #8614817 - Flags: review+
Keywords: checkin-needed
Well, Try looks good even when you repeat the right tests :)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/8156f7d328a7
https://hg.mozilla.org/mozilla-central/rev/7af2df7da870
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.