1478732 - Change nsHostResolver to dispatch one resolver task per native lookup

Status: NEW

(Core :: Networking, enhancement, P3)

Comment 1

[Valentin Gosu [:valentin] (he/him) {{ AFK in July }}]

Attachment: Bug 1478732 - Change nsHostResolver to dispatch one resolver task per native lookup

Comment 2

[Phabricator Automation]

Comment on attachment 8995257 [details]
Bug 1478732 - Change nsHostResolver to dispatch one resolver task per native lookup

Daniel Stenberg [:bagder] has approved the revision.

https://phabricator.services.mozilla.com/D2431

Comment 3

[Valentin Gosu [:valentin] (he/him) {{ AFK in July }}]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0cc38041c6fc2903d01cfcf6334b880c39e555b7

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9de74f5039a4
Change nsHostResolver to dispatch one resolver task per native lookup r=bagder

Comment 5

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/9de74f5039a4

Comment 6

[Valentin Gosu [:valentin] (he/him) {{ AFK in July }}]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ff11750d6c43dbbe420b64da03e6dde9fe3b22bb
Bug 1478732 - Backed out changeset 9de74f5039a4 r=backout

Comment 7

[Valentin Gosu [:valentin] (he/him) {{ AFK in July }}]

Reopening based on :mconley's mozregression.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/937c64cef999
Backed out changeset 9de74f5039a4 r=backout a=backout

Comment 9

[Laurence "GreenReaper" Parry]

I ran into the lag issue mentioned above. In case it matters, I have a 2011-era AMD dual-core netbook with about a hundred more tabs open than I should in five windows; even if they're not always active, if you're relying on some kind of background task completing in a timely manner, it might end up queued well after other tasks such as session saving.

Comment 10

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/ff11750d6c43

Comment 11

[Valentin Gosu [:valentin] (he/him) {{ AFK in July }}]

We do want to do this eventually, but it requires a bigger refactoring of the entire DNS code to be achievable.

Comment 12

[Valentin Gosu [:valentin] (he/him) {{ AFK in July }}]

Some context:

valentin: there were two issues with the bug. First the patch didn't properly dispatch a task every time it was supposed to, and sometimes DNS lookups would hang. Secondly, the threadpool wasn't exactly great for this, as we want the thread to pick up the request with the highest priority first
jens: Yeah, I assumed there to be some priority handling wanted that you'd need to resemble. Maybe we could have a prioritization wrapper around thread pool with three event queues or some such.

Right now the runnable event's priority can't really be used to express the priority of a DNS requests, especially since it can change while it's waiting in the queue.

Comment 13

[Valentin Gosu [:valentin] (he/him) {{ AFK in July }}]

Putting this in the backlog for now.
It would be a nice improvement, but not one that is critical right now.