Closed Bug 1471280 Opened Last year Closed Last year

Do not start/stop nsHostResolver threads

Categories

(Core :: Networking, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: valentin, Assigned: valentin)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-triaged])

Attachments

(2 files)

Following bug 1426019, nsHostResolver uses regular nsIThread for it's resolver workers. These threads don't really need to be shutdown, as we can just post events to them as necessary.
Comment on attachment 8989837 [details]
Bug 1471280 - Use nsThreadPool for DNS resolver threads

https://reviewboard.mozilla.org/r/254798/#review261766
Attachment #8989837 - Flags: review?(daniel) → review+
Comment on attachment 8989838 [details]
Bug 1471280 - Add new pref for how much longer resolver threads should remain idle

https://reviewboard.mozilla.org/r/254800/#review261770

::: modules/libpref/init/all.js:2046
(Diff revision 1)
>  // Contols whether or not "localhost" should resolve when offline
>  pref("network.dns.offline-localhost", true);
>  
> +// Defines how much longer resolver threads should stay idle before are shut down.
> +// A negative value will keep the thread alive forever.
> +pref("network.dns.resolver-thread-extra-idle-time-seconds", 60);

My gut feeling says we could use a little longer default timeout so that the threads would mostly linger and stick around while a user is actually active and only end them after a longer pause (like perhaps a 300 seconds timeout instead). I'm thinking 60 seconds is just a long page load and reading a page for a while.

Again, not based on science at all. Feel free to ignore if you disagree.
Attachment #8989838 - Flags: review?(daniel) → review+
(In reply to Daniel Stenberg [:bagder] from comment #4)
> > +pref("network.dns.resolver-thread-extra-idle-time-seconds", 60);
> 
> My gut feeling says we could use a little longer default timeout so that the
> threads would mostly linger and stick around while a user is actually active
> and only end them after a longer pause (like perhaps a 300 seconds timeout
> instead). I'm thinking 60 seconds is just a long page load and reading a
> page for a while.
> 
> Again, not based on science at all. Feel free to ignore if you disagree.

Well, this actually increases the idle time by 60 seconds above what it is now:
LongIdleTimeoutSeconds  300
ShortIdleTimeoutSeconds 60 

I want to run this for a couple of weeks, then I'm considering increasing this to forever™, according to bug 1470907 comment 7.
This could show a slight improvement in DNS telemetry, since threads don't have to be spun up now. But I have no idea how the extra threads would affect resource consumption on Android for example.
Pushed by valentin.gosu@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/db553cb4218d
Use nsThreadPool for DNS resolver threads r=bagder
https://hg.mozilla.org/integration/autoland/rev/d7788f2fd8a2
Add new pref for how much longer resolver threads should remain idle r=bagder
https://hg.mozilla.org/mozilla-central/rev/db553cb4218d
https://hg.mozilla.org/mozilla-central/rev/d7788f2fd8a2
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Depends on: 1498782
Duplicate of this bug: 463724
Depends on: 1500861
You need to log in before you can comment on or make changes to this bug.