Closed Bug 1260218 Opened 8 years ago Closed 8 years ago

SocketTransportService Socket Limits

Categories

(Core :: Networking, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: mcmanus, Assigned: mcmanus)

References

Details

(Whiteboard: [necko-active])

Attachments

(1 file)

Necko sets a target pool of 550 sockets and limits the http pool to 256 - which I suspect is too low a good fraction of the time these days. The failure mode would involve some waiting.

On linux/android/osx we can use pretty much any limit and be guided by getrlimit/setrlimit in trying to use it safely (or take what we can get).

otoh windows winsock does also provide some runtime guidance, but I've heard and read its hard to trust it. Most of the reported problems are win95 related. Still, there isn't a lot of good reason to not go up to 1000 which is the point where problems are rumored to kick in.

For unity - let's try 1000 (with 900 http) everywhere.

also add telemetry to see when we hit 1000/900 and how often we exceed 256. If the new level turns out to be a bottleneck, we can look for a different strategy.

For android its ok to change the socket level in sockettransportservice, but keep the http pref at 256.
Assignee: nobody → mcmanus
Whiteboard: [necko-active]
Attachment #8735930 - Flags: review?(dd.mozilla)
Comment on attachment 8735930 [details] [diff] [review]
SocketTransportService socket expansion

Review of attachment 8735930 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/base/nsSocketTransportService2.cpp
@@ +1479,5 @@
>      setrlimit(RLIMIT_NOFILE, &rlimitData);
> +    if ((getrlimit(RLIMIT_NOFILE, &rlimitData) != -1) &&
> +        (rlimitData.rlim_cur > SOCKET_LIMIT_MIN)) {
> +        gMaxCount = rlimitData.rlim_cur;
> +    }

The logic changed a bit here. I am not against it, just asking if it is intentional. Maybe set it to max SOCKET_LIMIT_TARGET. Probably it is ok to leave it like this.
Attachment #8735930 - Flags: review?(dd.mozilla) → review+
Comment on attachment 8735930 [details] [diff] [review]
SocketTransportService socket expansion

Review of attachment 8735930 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/base/nsSocketTransportService2.cpp
@@ +1479,5 @@
>      setrlimit(RLIMIT_NOFILE, &rlimitData);
> +    if ((getrlimit(RLIMIT_NOFILE, &rlimitData) != -1) &&
> +        (rlimitData.rlim_cur > SOCKET_LIMIT_MIN)) {
> +        gMaxCount = rlimitData.rlim_cur;
> +    }

the 'extra 250' was removed - that was intentional.

are you talking about something else? Because that would probably be unintentional.
(In reply to Patrick McManus [:mcmanus] from comment #4)
> Comment on attachment 8735930 [details] [diff] [review]
> SocketTransportService socket expansion
> 
> Review of attachment 8735930 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: netwerk/base/nsSocketTransportService2.cpp
> @@ +1479,5 @@
> >      setrlimit(RLIMIT_NOFILE, &rlimitData);
> > +    if ((getrlimit(RLIMIT_NOFILE, &rlimitData) != -1) &&
> > +        (rlimitData.rlim_cur > SOCKET_LIMIT_MIN)) {
> > +        gMaxCount = rlimitData.rlim_cur;
> > +    }
> 
> the 'extra 250' was removed - that was intentional.
> 
> are you talking about something else? Because that would probably be
> unintentional.

That is what I was talking about and that it can be bigger than SOCKET_..._TARGET.
But I do not see a real problem here, I am not against it.
https://hg.mozilla.org/mozilla-central/rev/7ce3240aca9f
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
The 256FD telemetry, from 4/28 to 5/22 on aurora48, was positive on 56k of 3.1M - which is almost 2% of sessions. The 900FD limit is at 0.

So the change appears to have alleviated a real bottleneck.
the telemetry also shows .2% of the dynamic maxcount tests on windows capping things between 50 and 169 (probably either 100 or 128) with 99.8 at 1000. That's a significant little bump - 2 in 1000 have problems with large numbers of connections.
Blocks: 1275054
You need to log in before you can comment on or make changes to this bug.