1999691 - Improve DNS cache eviction strategy

Status: REOPENED

(Core :: Networking, enhancement, P2)

Description

[Valentin Gosu [:valentin] (he/him)]

mozilla::net::HostRecordQueue::AddToEvictionQ adds entries to the evictionQ, and those entries get evicted when the queue is full.
We may remove it from the queue when mozilla::net::HostRecordQueue::MaybeRenewHostRecord gets called, which would cause it to be added back, but otherwise it's a FIFO eviction strategy.
Potential improvements:

• When a DNS cache entry is used, take it out and readd it to the eviction queue (ie switch to a least-recently-used eviction strategy)
• When an eviction happens, evict expired resources first considering their TTL.

Comment 1

[Max Inden]

If I understand correctly, we currently set a limit of 800 entries. Under the assumption, that each entry is < 1_000 bytes, this limit seems small.

@Valentin, would a third "potential improvement", namely increasing this limit be worth exploring as well?

Comment 2

[Oskar Mansfeld]

FYI the current limit of 800 was already doubled from 400 as part of the dns caching experiments in Bug 1950852. We didn't see any side effects there, but also no benefits (at least not through the telemetry noise).

Also for reference there is the old Bug 1463374 which was basically addressing the same thing and if I remember correctly from looking at it last year the suggested changes were also a combination of Valentin's recommendation, i.e. least-recently-used + evict expired first.

Comment 3

[Valentin Gosu [:valentin] (he/him)]

Increasing the limit even higher probably wouldn't hurt, but the switch to LRU is much more impactful.

Comment 4

[Kershaw Chang [:kershaw]]

Attachment: Bug 1999691 - When a DNS cache entry is used, take it out and readd it to the eviction queue, r=#necko

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ffb7bd5a1778
https://hg.mozilla.org/integration/autoland/rev/9ee64609489d
When a DNS cache entry is used, take it out and readd it to the eviction queue, r=necko-reviewers,valentin

Comment 6

[Cosmin Sabou [:CosminS]]

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/7496c8515212
https://hg.mozilla.org/mozilla-central/rev/7c6e6dc03dc0
Revert "Bug 1999691 - When a DNS cache entry is used, take it out and readd it to the eviction queue, r=necko-reviewers,valentin" for causing a top crash tracked under Bug 2004939.

Comment 8

[Cosmin Sabou [:CosminS]]

Backed out of central: https://hg.mozilla.org/mozilla-central/rev/7c6e6dc03dc0fd5049e5cba70fcf669cb2e772d7