NrIceCtx::Create() has too many bools; convert to flags
Categories
(Core :: WebRTC: Networking, defect, P2)
Tracking
()
backlog | tech-debt |
People
(Reporter: ekr, Assigned: bwc)
References
Details
Attachments
(1 file)
47 bytes,
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
RyanVM
:
approval-mozilla-esr78+
|
Details | Review |
Updated•9 years ago
|
Comment 1•7 years ago
|
||
Assignee | ||
Comment 2•4 years ago
|
||
Let's go ahead and do this. We're grabbing config values all over the place, and it is getting hard to read.
Assignee | ||
Comment 3•4 years ago
•
|
||
Assignee | ||
Comment 4•4 years ago
|
||
Assignee | ||
Comment 5•4 years ago
|
||
Assignee | ||
Comment 6•4 years ago
|
||
Assignee | ||
Comment 7•4 years ago
|
||
Assignee | ||
Comment 8•4 years ago
|
||
Assignee | ||
Comment 9•4 years ago
|
||
Assignee | ||
Comment 10•4 years ago
|
||
Try looks ok.
Assignee | ||
Comment 11•4 years ago
|
||
Assignee | ||
Comment 12•4 years ago
|
||
Comment 13•4 years ago
|
||
Comment 14•4 years ago
|
||
Backed out for bustages on ice_unittest.cpp
Backout link: https://hg.mozilla.org/integration/autoland/rev/99f760e933bfbcace9877fe76c81781caf3e9266
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=308928864&repo=autoland&lineNumber=27383
Assignee | ||
Comment 15•4 years ago
|
||
Huh. The only failure is in a base-toolchains build, which seems to be intended to quickly catch cases where one of our "real" toolchains is insufficiently modern. However, all of the other compilations are happy; do we need to bump one of the toolchains in the base-toolchains job?
Assignee | ||
Comment 16•4 years ago
•
|
||
It does not look like gcc 7 is supported anymore (by the GNU project). Is that where we're tripping up? Does gcc 8 support this syntax?
Assignee | ||
Comment 17•4 years ago
|
||
So it looks like the one holdout for gcc 7 is ccov builds (which weren't run on the autoland push for this bug). Those probably could use gcc 8?
Assignee | ||
Comment 18•4 years ago
|
||
Assignee | ||
Comment 19•4 years ago
•
|
||
It looks like the base-toolchain builds are unhappy with some code in other places if they use gcc 8, although gcc 8 seems to be ok with the syntax I'm using here. I guess I'll have to fall back to the clunkier syntax.
Assignee | ||
Comment 20•4 years ago
|
||
Assignee | ||
Comment 21•4 years ago
|
||
Waiting for some retriggers, but try looks good so far.
Comment 22•4 years ago
|
||
Comment 23•4 years ago
|
||
bugherder |
Comment 24•4 years ago
|
||
Byron, does this improvement makes sense?
== Change summary for alert #26495 (as of Mon, 13 Jul 2020 10:30:01 GMT) ==
Improvements:
55% build times linux64-shippable opt instrumented taskcluster-m5.4xlarge 2,225.72 -> 1,002.70
52% build times osx-shippable opt instrumented taskcluster-m5.4xlarge 2,401.25 -> 1,149.88
49% build times linux64-shippable opt instrumented taskcluster-c5d.4xlarge 2,137.52 -> 1,098.07
47% build times osx-shippable opt instrumented taskcluster-c5.4xlarge 2,312.68 -> 1,225.96
45% build times android-4-0-armv7-api16 pgo instrumented taskcluster-c5d.4xlarge 1,767.90 -> 969.43
42% build times osx-shippable opt instrumented taskcluster-c5d.4xlarge 2,318.09 -> 1,334.99
41% build times android-4-0-armv7-api16 pgo instrumented taskcluster-c5.4xlarge 1,807.32 -> 1,070.22
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=26495
Comment 25•4 years ago
|
||
No, those come from bug 1637544.
Assignee | ||
Comment 27•4 years ago
|
||
Comment on attachment 9161903 [details]
Bug 1193437: (79 backport) Use config structs in NrIceCtx, and centralize pref lookups some. r=mjf
Beta/Release Uplift Approval Request
- User impact if declined:
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky):
- String changes made/needed: None
Assignee | ||
Comment 28•4 years ago
|
||
Comment on attachment 9161903 [details]
Bug 1193437: (79 backport) Use config structs in NrIceCtx, and centralize pref lookups some. r=mjf
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration:
- User impact if declined:
- Fix Landed on Version: 80
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky):
- String or UUID changes made by this patch: None
Comment 29•4 years ago
|
||
Comment on attachment 9161903 [details]
Bug 1193437: (79 backport) Use config structs in NrIceCtx, and centralize pref lookups some. r=mjf
Approved for 79.0b9 and 78.1esr.
Comment 30•4 years ago
|
||
bugherder uplift |
Comment 31•4 years ago
|
||
Backed out for Beta build bustage:
https://hg.mozilla.org/releases/mozilla-beta/rev/8b8c4c23c0bcf047000ec959931f5df2b57a08ea
Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=310034497&repo=mozilla-beta&lineNumber=16606
Comment 33•4 years ago
|
||
Trivial to rebase around at least. I've got a green Try push and will re-land :)
Updated•4 years ago
|
Comment 34•4 years ago
|
||
bugherder uplift |
Comment 35•4 years ago
|
||
bugherder uplift |
Description
•