Closed
Bug 1381627
Opened 7 years ago
Closed 7 years ago
Label the EverySecondTelemetryCallback_m runnable
Categories
(Core :: WebRTC: Signaling, enhancement, P1)
Core
WebRTC: Signaling
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: billm, Assigned: ng)
References
Details
Attachments
(1 file)
This runnable happens pretty often. It will need to be labeled. It would be nice if we could use SystemGroup here. But it seems possible that we read data that needs to be read when no other runnables are running.
I also wonder if this runnable could be run at idle priority. That would solve the labeling issue and also improve performance.
Updated•7 years ago
|
Rank: 25
Component: WebRTC → WebRTC: Signaling
Priority: -- → P2
Comment 1•7 years ago
|
||
Up-ing priority as I learned this is part of the remaining 3% of unlabeled runnables we need to make the concurrent scheduler work effectively for project quantum.
Rank: 25 → 15
Priority: P2 → P1
Comment 4•7 years ago
|
||
Gentle ping here :)
Assignee | ||
Comment 5•7 years ago
|
||
Thanks for the poke.
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(na-g)
Assignee | ||
Comment 6•7 years ago
|
||
This looks like it labeled in Bug 1377222.
https://hg.mozilla.org/mozilla-central/rev/99566b93d105#l14.14
Flags: needinfo?(na-g)
Assignee | ||
Comment 7•7 years ago
|
||
Andrew, could you double check me on comment 6 before I close?
Flags: needinfo?(overholt)
Reporter | ||
Comment 8•7 years ago
|
||
That change just gave it a name. The problem here is the SetTarget call:
http://searchfox.org/mozilla-central/rev/5ce320a16f6998dc2b36e30458131d029d107077/media/webrtc/signaling/src/peerconnection/PeerConnectionCtx.cpp#371
We need the target to be the result of calling EventTargetFor on some SchedulerGroup: either the SystemGroup or a DocGroup or TabGroup. If we use the SystemGroup, that means that the timer code is not allowed to modify any state in a way that would be observable by web content. If we use a DocGroup or TabGroup, that means that the timer can modify state in a way that is observable only by the document or tab in question.
So the main issue here is what EverySecondTelemetryCallback_m is actually doing and whether it modifies state in an observable way.
Flags: needinfo?(overholt)
Assignee | ||
Comment 9•7 years ago
|
||
Assignee | ||
Comment 10•7 years ago
|
||
Updated•7 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8902310 -
Flags: review?(wmccloskey)
Attachment #8902310 -
Flags: review?(jib)
Comment 13•7 years ago
|
||
mozreview-review |
Comment on attachment 8902310 [details]
Bug 1381627 - EverySecondTelemetryCallback target
https://reviewboard.mozilla.org/r/173842/#review180180
lgtm.
Attachment #8902310 -
Flags: review?(jib) → review+
Assignee | ||
Comment 14•7 years ago
|
||
(In reply to Andrew Overholt [:overholt] from comment #12)
> Is this landable? :)
Yes, review pending.
Reporter | ||
Comment 15•7 years ago
|
||
mozreview-review |
Comment on attachment 8902310 [details]
Bug 1381627 - EverySecondTelemetryCallback target
https://reviewboard.mozilla.org/r/173842/#review180654
Attachment #8902310 -
Flags: review?(wmccloskey) → review+
Comment 16•7 years ago
|
||
Pushed by na-g@nostrum.com:
https://hg.mozilla.org/integration/autoland/rev/ac5161ac285d
EverySecondTelemetryCallback target r=billm,jib
Comment 17•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(na-g)
You need to log in
before you can comment on or make changes to this bug.
Description
•