Change default of network.http.max-urgent-start-excessive-connections-per-host to 3

RESOLVED FIXED in Firefox 55

Status

()

Core
Networking: HTTP
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: mayhemer, Assigned: Amy)

Tracking

(Blocks: 1 bug)

Trunk
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [necko-active])

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

a year ago
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

a year ago
Created attachment 8849047 [details] [diff] [review]
implementation

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

a year 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

a year ago
Created attachment 8849434 [details] [diff] [review]
implementation

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

a year 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+
(Reporter)

Comment 5

a year ago
Amy, any reason not to land this now?
Flags: needinfo?(amchung)
(Assignee)

Comment 6

a year ago
Created attachment 8851677 [details] [diff] [review]
implementation

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

a year ago
Keywords: checkin-needed
(Reporter)

Comment 7

a year ago
Aha, no problem :)
Needs rebasing.
Flags: needinfo?(amchung)
Keywords: checkin-needed
(Assignee)

Updated

a year ago
Keywords: checkin-needed
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

a year ago
Created attachment 8852116 [details] [diff] [review]
implementation

Already rebased the latest changes on nsHttpConnectionMgr.cpp.
Attachment #8852017 - Attachment is obsolete: true
Attachment #8852116 - Flags: review+
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 12

a year 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

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b8a8765f0474
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.