Closed
Bug 1348041
Opened 8 years ago
Closed 8 years ago
Change default of network.http.max-urgent-start-excessive-connections-per-host to 3
Categories
(Core :: Networking: HTTP, enhancement)
Core
Networking: HTTP
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: mayhemer, Assigned: amchung)
References
(Blocks 1 open bug)
Details
(Whiteboard: [necko-active])
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
amchung
:
review+
|
Details | Diff | Splinter Review |
To mitigate bug 1345845 let's lower the number of excessive urgent start connections.
Amy, can you do it please? I missed during the last review that you didn't change the value to 4 as I wanted. And when we are here, I think 3 connections will do. Thanks.
Assignee | ||
Comment 1•8 years ago
|
||
Hi Honza,
Sorry I didn't change the default of max-urgent-start-excessive-connections-per-host, and I finished to modify it on the patch:
1. Modified the default of network.http.max-urgent-start-excessive-connections-per-host to 3 on all.js
2. Modified the defalut of mMaxUrgntStartQ to 3 on constructor nsHttpHandler().
Would you help me to review my patch?
Thanks!
Attachment #8849047 -
Flags: review?(honzab.moz)
Reporter | ||
Comment 2•8 years ago
|
||
Amy, I have one more thing. The members and some argument should also be renamed from MaxUrgentQ to something more reflecting the new meaning of the preference. Like MaxUrgentExcessiveConns or something. Would you please do it as part of this patch too?
Flags: needinfo?(amchung)
Assignee | ||
Comment 3•8 years ago
|
||
Hi Honza,
I have replaced all MaxUrgent to MaxUrgentExcessiveConns.
Would you help me to review my patch?
Thanks!
Attachment #8849047 -
Attachment is obsolete: true
Attachment #8849047 -
Flags: review?(honzab.moz)
Flags: needinfo?(amchung)
Attachment #8849434 -
Flags: review?(honzab.moz)
Reporter | ||
Comment 4•8 years ago
|
||
Comment on attachment 8849434 [details] [diff] [review]
implementation
Review of attachment 8849434 [details] [diff] [review]:
-----------------------------------------------------------------
Thank you!
Attachment #8849434 -
Flags: review?(honzab.moz) → review+
Assignee | ||
Comment 6•8 years ago
|
||
Hi Honza,
Sorry for I was waiting for the testing result of try server.
Thanks!
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f60693a7da7134b87d4f425989ba6819404f56b&selectedJob=86642360
Attachment #8849434 -
Attachment is obsolete: true
Flags: needinfo?(amchung)
Attachment #8851677 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 7•8 years ago
|
||
Aha, no problem :)
Assignee | ||
Comment 9•8 years ago
|
||
Attachment #8851677 -
Attachment is obsolete: true
Flags: needinfo?(amchung)
Attachment #8852017 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 10•8 years ago
|
||
patching file netwerk/protocol/http/nsHttpConnectionMgr.cpp
Hunk #3 FAILED at 1031
1 out of 4 hunks FAILED -- saving rejects to file netwerk/protocol/http/nsHttpConnectionMgr.cpp.rej
I'm guessing it was changes on inbound that only recently got merged around?
Keywords: checkin-needed
Assignee | ||
Comment 11•8 years ago
|
||
Already rebased the latest changes on nsHttpConnectionMgr.cpp.
Attachment #8852017 -
Attachment is obsolete: true
Attachment #8852116 -
Flags: review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 12•8 years ago
|
||
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8a8765f0474
Change default value of network.http.max-urgent-start-excessive-connections-per-host to 3. r=mayhemer
Keywords: checkin-needed
Comment 13•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•