1672520 - First step in refactoring nsHttpConnectionMgr

Status: RESOLVED FIXED

(Core :: Networking: HTTP, task, P2)

Comment 1

[Dragana Damjanovic [:dragana]]

Attachment: Part 0 - Move all different classes defined in nsHttpConnectionMgr into separate files.

Comment 2

[Dragana Damjanovic [:dragana]]

Attachment: Part 1 - Move Pending queues and their logic into a separate class.

Comment 3

[Dragana Damjanovic [:dragana]]

Attachment: Part 2 - Make mIdleConns privat

Comment 4

[Dragana Damjanovic [:dragana]]

Attachment: Part 2 - Make mIdleConns private

Comment 5

[Dragana Damjanovic [:dragana]]

Attachment: Part 3 - Make mActiveConns private

Comment 6

[Dragana Damjanovic [:dragana]]

Attachment: Part 4 - Avoid changing counters when a conn is moved from one to another ConnectionEntry

Comment 7

[Dragana Damjanovic [:dragana]]

Attachment: Part 5 - Move PruneDeadConnectionsAfter call where the count is incremented so that everything is at one place

Comment 8

[Dragana Damjanovic [:dragana]]

Attachment: Part 6 - Make HalfOpens private

Comment 9

[Dragana Damjanovic [:dragana]]

Attachment: Make member variable of PendingTransactionInfo private

Comment 10

[Dragana Damjanovic [:dragana]]

This is just the first step. nsHttpConnectionMgr can be improved further.

Comment 11

[Dragana Damjanovic [:dragana]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=76620eef6cdd12cf71a9a21f84c464671934982d

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e2a2ecf84209
Part 0 - Move all different classes defined in nsHttpConnectionMgr into separate files. r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/473498b3df4c
Part 1 - Move Pending queues and their logic into a separate class. r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/c1b12059f317
Part 2 - Make mIdleConns private r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/6a0a89a02779
Part 3 - Make mActiveConns private r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/09de9356d225
Part 4 - Avoid changing counters when a conn is moved from one to another ConnectionEntry r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/84ad3077d7ef
Part 5 - Move PruneDeadConnectionsAfter call where the count is incremented so that everything is at one place r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/dd9c9c3c959c
Part 6 - Make HalfOpens private r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/fd7e69c065d9
Make member variable of PendingTransactionInfo private r=necko-reviewers,valentin

Comment 13

[Dorel Luca [:dluca]]

Backed out 8 changesets (bug 1672520) for Build bustage in gecko/netwerk/protocol/http/nsHttpConnectionMgr.cpp. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer?job_id=320359764&repo=autoland&lineNumber=46313

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=fd7e69c065d928cdb2df97fe64e800de7ece245c

Backout:
https://hg.mozilla.org/integration/autoland/rev/589b45ea4b87f0378ddb9e31e149e70fa301bc7e

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e81676ee1501
Part 0 - Move all different classes defined in nsHttpConnectionMgr into separate files. r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/8a236eb4833f
Part 1 - Move Pending queues and their logic into a separate class. r=necko-reviewers,kershaw
https://hg.mozilla.org/integration/autoland/rev/c83020f6bf92
Part 2 - Make mIdleConns private r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/378ed0b660f7
Part 3 - Make mActiveConns private r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/279fd8b1210d
Part 4 - Avoid changing counters when a conn is moved from one to another ConnectionEntry r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/bbb1da52a501
Part 5 - Move PruneDeadConnectionsAfter call where the count is incremented so that everything is at one place r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/12021a9f2d20
Part 6 - Make HalfOpens private r=necko-reviewers,valentin
https://hg.mozilla.org/integration/autoland/rev/cac5627ac1ad
Make member variable of PendingTransactionInfo private r=necko-reviewers,valentin

Comment 15

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/e81676ee1501
https://hg.mozilla.org/mozilla-central/rev/8a236eb4833f
https://hg.mozilla.org/mozilla-central/rev/c83020f6bf92
https://hg.mozilla.org/mozilla-central/rev/378ed0b660f7
https://hg.mozilla.org/mozilla-central/rev/279fd8b1210d
https://hg.mozilla.org/mozilla-central/rev/bbb1da52a501
https://hg.mozilla.org/mozilla-central/rev/12021a9f2d20
https://hg.mozilla.org/mozilla-central/rev/cac5627ac1ad

Comment 16

[Dragana Damjanovic [:dragana]]

This has been fixed.

Comment 17

[Alexandru Ionescu (needinfo me) [:alexandrui]]

I tried a local backout of bug 1672520 but it failed for some reason, so I need to ask you if the regression below makes sense to be caused by your patches?
== Change summary for alert #27601 (as of Wed, 11 Nov 2020 13:38:20 GMT) ==

Regressions:

Ratio	Suite	Test	Platform	Options	Absolute values (old vs new)
8%	imdb	SpeedIndex	linux64-shippable-qr	cold webrender	960.94 -> 1,038.58
8%	imdb	SpeedIndex	linux64-shippable	cold	956.16 -> 1,031.67

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=27601