filtering: fix engine rebuild memory usage

[vortexilation]

Fixes #8297.
Fixes #8491.

Diagnosis

Both issues report AdGuard Home being OOM-killed, and both blame a release — #8297 blames
v0.107.73, #8491 blames v0.107.78. Nothing under internal/filtering changed in either release (verified against
v0.107.72...v0.107.73 and v0.107.77...v0.107.78), so neither is a regression in this
package.

That claim is deliberately limited to this package, because a dependency did change:
v0.107.78 updated urlfilter from v0.23.2 to v0.23.4, which grew rules.NetworkRule from
264 to 296 bytes. On a 2.1M-rule list those 32 bytes per rule are about 64 MiB of extra
steady-state footprint, so the bump plausibly contributes to the reports against that
release, though it cannot account for a transient peak several times the steady state.
v0.107.73 is unaffected: urlfilter was v0.23.1 both before and after it, so #8297 has no
such explanation.

The trigger is rule lists growing; several commenters note HaGeZi's Threat Intelligence
Feeds roughly doubled in size over the same period.

The mechanism is the one the pprof profile in
@starterhomelab's comment
points at:

      flat  flat%        cum   cum%
   45.05MB 31.39%   141.84MB 98.85%  urlfilter.NewDNSEngine
   34.01MB 23.70%    34.01MB 23.70%  urlfilter/rules.NewNetworkRule
   32.29MB 22.50%    32.29MB 22.50%  urlfilter/internal/lookup.(*ShortcutIndex).Add
   15.50MB 10.80%    15.50MB 10.80%  urlfilter/rules.NewHostRule

DNSFilter.initFiltering must keep the previous engines alive while it builds the new ones,
since they serve DNS for the duration — so the live heap roughly doubles. With the default
GC target of 100% the heap is only collected once it has grown to twice the live heap, so
the transient peak of a rebuild ends up at about three times the memory a single set of
engines takes. That is what pushes 512 MB–1 GB installations over the edge, and it explains
why the reported peaks are far larger than the steady-state footprint while inuse_space
after a clean refresh barely moves.

Measurements

Rebuild of a single 2.1M-rule list, peak heap and rebuild duration:

GC target	peak (n CPUs)	peak (1 CPU)	time (n CPUs)	time (1 CPU)
100%	830 MiB	878 MiB	4.8 s	5.2 s
50%	665 MiB	713 MiB	4.7 s	5.1 s
20%	575 MiB	597 MiB	4.8 s	6.9 s

Steady state with one such engine live is 273 MiB.

Changes

1. Skip the rebuild when nothing has changed. initFiltering now compares a
fingerprint of the rule lists it is about to build from — list ID, file path, inline data,
and the contents of each backing file — against the one the current engines were built
from, and returns early when they match.

This is the largest practical win for a common class of reports in these threads. Every
filtering-related HTTP API handler calls EnableFilters, so setting any filtering
property rebuilt both engines even when the value was unchanged. That is why users running
AdGuardHome-Sync see a rebuild storm on every sync cycle, and why "unblock one domain in the
UI" could take a box down.

The contents are read in full rather than summarized by the size and the modification time
of each file. Those are metadata, not a content identity: a restored backup or an explicit
os.Chtimes can leave both unchanged while the rules differ, which would keep stale rules
in use, and merely touching a file whose contents are unchanged would force a needless
rebuild. Reading the lists is far cheaper than rebuilding the engines from them, which is
what this check exists to avoid.

2. Serialize rebuilds. Nothing previously prevented the periodic filter update and a
configuration-driven rebuild from running concurrently, which doubles an already-doubled
peak. A new initMu makes initFiltering mutually exclusive.

3. Lower the GC target for the duration of a rebuild (debug.SetGCPercent(20)),
cutting the peak by about a third. A rebuild is background work and DNS keeps being served
by the previous engines while it runs, so a slower rebuild is a much better outcome than an
OOM kill, which takes DNS down until an operator intervenes. The cost is confined to
single-CPU systems (+1.7 s in the table above); on multi-core it is free.

The target is only ever tightened — a user who has set a stricter GOGC, or disabled the
GC entirely, keeps their setting.

20 is the knee of the curve, not a round number. Peak resident memory and rebuild duration
against the target, on a 2.1M-rule list:

GC target	peak (n CPUs)	peak (1 CPU)	time (n CPUs)	time (1 CPU)
100%	490 MiB	564 MiB	4.8 s	5.1 s
50%	406 MiB	—	4.7 s	—
20%	351 MiB	360 MiB	4.9 s	7.2 s
10%	331 MiB	339 MiB	5.2 s	10.2 s
5%	326 MiB	342 MiB	6.6 s	12.0 s

Below 20 the peak stops moving and the duration does not: on one CPU, 10% buys 6% of peak
for 42% more time, and 5% buys nothing at all while costing 67%.

These two tables and the one under change 5 come from a standalone harness over a
synthetic 2.1M-rule list, which is cheaper per rule than a real blocklist — one engine is
118 MiB live here against 273 MiB for the real list in the first table above. The absolute
figures are therefore lower than that table's throughout and the two are not directly
comparable; the ratios are what carry over, and the 100%-to-20% reduction agrees between
them.

5. Collect before the rebuild as well as after it. Tightening the GC target decides
when the next collection happens; it does not clear what is already dead. So a rebuild
began on top of whatever heap serving DNS had left behind and carried it resident through
its early phase, adding that garbage to the peak in full. Entering the same rebuild with
120 MiB of dead objects on the heap:

	peak (n CPUs)	peak (1 CPU)	time (n CPUs)	time (1 CPU)
collect after	372.8 MiB	400.3 MiB	4.8 s	8.3 s
collect before	350.2 MiB	362.5 MiB	4.8 s	8.7 s

6% off the peak on multi-core and 9% on one CPU, for 0.4 s on one CPU and nothing on
multi-core. Scaled against the reporter's 763 MB peak that is roughly 45-70 MB, on a device
whose margin got as low as 11 MB — though how much garbage is actually on the heap when a
rebuild starts depends on the query load, so 120 MiB is an assumption, not a measurement
from that device.

4. Release resources when a rebuild fails. If newRuleStorage failed partway, or the
allow-list storage failed after the block-list storage was already built, the opened rule
lists were dropped without Close(), leaking their file descriptors — and the early return
skipped debug.FreeOSMemory(), leaving the failed rebuild's peak resident. Both are now
handled, and FreeOSMemory runs on every path.

Tests

New tests in internal/filtering/initfiltering_internal_test.go cover:

• fingerprint stability, and sensitivity to changed data, changed ID, swapped allow/block
  groups, an added list, changed file contents, a changed mtime at equal size, and a removed
  file;
• that an unchanged filter set does not rebuild and a changed one does;
• that a failed rebuild leaves the previous engines installed and does not record itself
  as the current state, so the next rebuild still runs;
• that a rebuild collects both before and after itself, and that a skipped one does
  neither.

go test ./..., go vet ./internal/... and gofmt are clean, and the filtering package
passes under -race.

Review updates

Addressing @Sil3ntVip3r's review:

• Hash the rule-list contents, not their metadata. Size plus mtime is not a content
  identity, so a file replaced by different rules of the same length with the modification
  time preserved would not change the fingerprint, and initFiltering would keep serving the
  previous rules. hashFileMeta is now hashFileContents, streaming the file through the
  same digest.

  Reading every list on every filtering-related API request then cost about 22% more
  latency per configuration push, and almost all of it was the digest rather than the read
  — over the 43 MiB list, best of five warm: read only 11.2 ms, read + SHA-256 123.6 ms,
  read + maphash 11.3 ms. The fingerprint never leaves the process and is only compared
  against the one the current engines were built from, so it needs no second-preimage
  resistance, and it is seeded once per process so a collision cannot be prepared inside a
  rule list. A collision would cost a skipped rebuild and nothing worse. So it is
  hash/maphash now, and the check costs the read alone.

• Serialize the changes of the garbage-collection target. initMu belongs to one
  DNSFilter while debug.SetGCPercent changes the whole process, so two overlapping
  rebuilds restored each other's saved value and could leave the process at the stricter
  target permanently. The bookkeeping moved into a package-level gcTarget that counts the
  rebuilds in flight and only restores after the last one finishes. Its setter is injectable,
  so the overlap is covered deterministically instead of by mutating the test process's own
  target.

Both are pinned by tests that fail against the previous implementation: different bytes with
identical metadata, identical bytes with a changed mtime, and a two-instance overlap. Note
that the existing same_size_new_mtime case asserted the old behaviour and became
same_bytes_new_mtime.

The diagnosis wording is narrowed as well: the "no code change" claim now applies only to
internal/filtering, with the urlfilter v0.23.2 to v0.23.4 bump and the resulting
NetworkRule growth called out. Confirmed independently here: 264 to 296 bytes on 64-bit,
and unchanged across v0.107.72 to v0.107.73, so the caveat applies to #8491 and not #8297.

Field validation

Confirmed on a 1 GB device against a 2.1M-rule list by a reporter in
#8491:
no OOM, the process survived the reload with the same PID, peak RSS 763 MB against a
365 MB baseline, settling back to 423 MB. Seven heap profiles from that reload
(analysis)
show both mechanisms firing on real hardware:

• The GC target was tightened for the rebuild and restored afterwards: NextGC/Alloc is
  1.11 during the rebuild and 2.00 after it.
• The deferred debug.FreeOSMemory() ran exactly once, moving HeapReleased from 2.4 MiB
  to 368.8 MiB in one step.

The same profiles put 96.4% of the live heap in the two coexisting urlfilter engines
(290.3 MiB old plus 266.3 MiB new) and everything else in the process at 19.5 MiB, so
there is no remaining allocation of any size on the AdGuard Home side of the rebuild.

They did show one avoidable cost on a path this PR adds, fixed in
filtering: pool the fingerprint read buffer: the fingerprint reads each rule list through
io.Copy, which delegates to os.File.WriteTo and allocates a fresh 32 KiB buffer per list
per call. It retains nothing, but this runs on every filtering-related API request, which is
the path that exists to be cheap.

BenchmarkDNSFilter_initFiltering/unchanged_10000
before   33706 B/op   9 allocs/op
after      889 B/op   9 allocs/op

Multi-cycle soak

The same reporter kept the build and the large list running and reported again on
12 August,
this time across repeated refresh cycles rather than a single event:

• No OOM for the duration, with the large HaGeZi list active throughout.
• Roughly 10-12 refresh cycles, counted from each list's own ~24h interval.
• The OS thread count went 10 → 11 → 12 → 12 → 13 and then held, four days flat at 13 by
  the time of the report.

The thread figure was the open question left by the heap analysis above. The shape is what
matters: the Go runtime parks threads rather than destroying them, so the count ratchets to
a high-water mark and stops, and a real leak would look like one more thread per rebuild
that never plateaus. It plateaued. The GOMAXPROCS + 5 figure I quoted earlier was a poor
yardstick, though — this build starts at 10 threads on a clean launch of a four-core
device, so the threshold as written was already passed before any rebuild happened.

What the run does not show, stated plainly:

• It is a soak on a composite build that also carries dnsforward: fix misreported upstream response times #8522 and dnsproxy#520. The isolated
  evidence for this PR remains the A/B benchmark on frozen list bytes
  (comment):
  peak RSS 890 MiB on the parent commit against 298 MiB on this branch, one tree, one
  toolchain, alternating order.
• The cycle count is derived from the configured refresh intervals, not from observed
  rebuilds. HaGeZi's GitHub was unreachable for several days at the end of the run, which
  left the copies AdGuard Home serves frozen — a commenter
  verified
  the file header stuck at 9 August. Refreshes in that window fetched unchanged bytes and
  were short-circuited by change 1 rather than rebuilding anything. That is the intended
  behaviour and it means the path got real exercise, but the number of full engine rebuilds
  actually covered is lower than the cycle count.
• The reporter runs the large list plus a modest set, not the large list alongside
  everything else, so this is not a worst-case configuration.

One number from that report deserves not to be glossed over: the tightest margin in the
whole run was about 11 MB of MemAvailable, and it came during one of the smaller
evening list updates rather than during the large list's own rebuild. That is expected —
initFiltering rebuilds every engine from every list whenever any one of them changes, so
the peak is set by the total rule count and not by which list moved — and chasing it is
what produced change 5 above. But it is worth being clear about what this PR buys. It
removes the rebuild peak that was producing the OOM kills; it does not make a 1 GB device
comfortable.

The rest is structural, and I looked for more before saying so. The GC target is at the
knee of its curve, as the table under change 3 shows. A soft memory limit does go lower —
315 MiB against 351 MiB for GOGC=20 on the same rebuild — but only when it is sized
correctly: at 260 MiB the same rebuild took 17.6 s instead of 4.9 s and still overshot the
limit. AdGuard Home cannot know the right number, since it depends on the device and on
everything else running there, which is exactly why GOMEMLIMIT belongs to the operator
rather than to this code. Beyond that, the heap profiles put 96.4% of the peak in two
urlfilter engines that have to coexist for the swap.

Notes for reviewers

Two pre-existing problems I noticed in this code but deliberately left out of scope:

• handleFilteringSetRules writes d.conf.UserRules without holding conf.filtersMu,
  racing the read in enableFiltersLocked.
• periodicallyRefreshFilters computes its network-error backoff as
  ivl = max(ivl, maxInterval), which jumps straight to the one-hour maximum on the first
  error; the surrounding code reads as though min was intended.

Happy to adjust the GC target, split the commit, or drop any of the four parts if you'd
prefer a narrower change.

[Sil3ntVip3r]

I found two correctness edge cases in the current patch. Both are local to the new fingerprint/GC behavior and have focused fixes.

[Sil3ntVip3r]

Re-reviewed the current head 7df0afac. Both correctness issues from my
earlier review are addressed:

• The fingerprint now follows file contents rather than size and mtime.
• Process-global GC-target changes are coordinated across overlapping
  DNSFilter instances and covered deterministically.

I also profiled this revised head against master (5f6cb57d) using synthetic
unique rule sets of 10k, 50k, and 100k rules on macOS ARM64. At 100k rules:

• Maximum RSS fell from 89.7 MiB to 54.8 MiB (-39%).
• Sampled changed-rebuild peak fell from 47.8 MiB to 34.5 MiB (-28%), with
  about 23% longer rebuild time.
• An unchanged rebuild fell from 59.6 ms / 63.86 MiB allocated to 1.22 ms /
  0.033 MiB.
• Concurrent-rebuild peak fell from 70.7 MiB to 35.2 MiB (-50%), with the
  expected serialization latency.
• Post-GC retained heap was effectively unchanged at about 15.17 MiB, and six
  alternating rebuilds did not reproduce permanent heap growth.

The measurements support the patch's intended transient-memory and
redundant-work improvements. They do not substitute for the reporter's 2.1M
rule list or Linux/OpenWrt/LXC validation.

One diagnosis wording caveat: v0.107.77 to v0.107.78 updated urlfilter from
v0.23.2 to v0.23.4, and NetworkRule grew from 264 to 296 bytes. So it is
accurate to say that nothing under internal/filtering changed, but the
broader statement that there was no filtering-related code change should
remain narrower; the dependency change could contribute to steady memory,
although it does not explain the full transient spike.

The two requested review fixes are resolved, and the revised implementation is
well supported by the bounded profiling evidence.

[vortexilation]

@Sil3ntVip3r thanks for the re-review and for the independent profiling — the diagnosis wording is now narrowed in the description, and I confirmed your dependency finding locally: rules.NetworkRule is 264 bytes under urlfilter v0.23.2 and 296 under v0.23.4. Worth adding that it applies to #8491 only — urlfilter was v0.23.1 on both sides of v0.107.73, so #8297 has no such explanation.

On the gap you flagged, here is Linux validation against a list of the reporter's size.

Setup

• List: HaGeZi Threat Intelligence Feeds, 2,158,753 rules, 42.6 MiB — the list several commenters in High memory and CPU usage #8297 identify as having roughly doubled.
• Compared: this PR's head against its own parent (8e56afa), so the delta isolates the patch rather than mixing in unrelated master changes.
• Host: Linux x86_64 (WSL2), glibc, 36 cores, 31 GiB RAM, GOMAXPROCS=36.
• Method: peak RSS read from VmHWM in /proc/self/status, reset via /proc/self/clear_refs between phases so each figure covers only its own phase. Page cache warmed before both runs.

Results

Phase	Parent 8e56afa	This PR	Δ peak
initial build	4.39 s / 422 MiB	5.58 s / 276 MiB	−35%
changed rebuild	5.17 s / 854 MiB	5.34 s / 564 MiB	−34%
unchanged rebuild	4.32 s / 874 MiB / 1276 MiB alloc	0.11 s / 314 MiB / 0 MiB alloc	−64%
concurrent rebuild	5.39 s / 1183 MiB	9.99 s / 579 MiB	−51%
steady-state RSS	404 MiB	318 MiB	−21%
overall peak	1183 MiB	579 MiB	−51%

The transient peak on a real rebuild drops by about a third, and the worst case over the whole run halves. Rebuild time costs +3% on a changed rebuild and +27% on the initial build, consistent with the ~23% you measured.

An unchanged rebuild goes from 4.3 s and 1.28 GiB allocated to 0.11 s and zero allocation — roughly 38x. That is the AdGuardHome-Sync case, and on a memory-constrained box it is the difference between a rebuild storm and nothing happening at all.

On the concurrent figure

My first attempt at that row was misleading, so it is worth saying how it is built. Parking the engines on one list and then racing two rebuilds lets the fingerprint skip one of them, which measures deduplication rather than serialization. The run above parks the engines on a third list first, so both racing calls differ from the current state and from each other and neither can be skipped. Both sides allocated the same 2551 MiB, confirming each performed two full rebuilds — so the halved peak is the serialization, and the 9.99 s against 5.39 s is its expected latency cost.

What this does not cover

This is glibc x86_64 with 31 GiB of RAM. It is not OpenWrt or LXC, not musl, not ARM, and not a 512 MiB–1 GiB memory ceiling — which is precisely where the OOM kills in these issues occur. The relative improvements should carry, but I have not demonstrated that on the platforms the reporters run, and the GC-target change is the part most likely to behave differently under a hard cgroup limit. Happy to run this on OpenWrt in a VM if that would help, or to hand over the harness so someone with the reporters' hardware can reproduce it.

[Sil3ntVip3r]

Re-reviewed the current head a6c55d8, including both benchmark commits added after my earlier review. The self-contained benchmark correctly alternates changed rule sets, keeps the unchanged case stable, and the helper extraction preserves those measurements. Local results on macOS ARM64 with one iteration per case were: changed 10k 6.78 MB / 60,209 allocs, unchanged 10k 33.3 KB / 9 allocs; changed 100k 65.4 MB / 600,851 allocs, unchanged 100k 33.3 KB / 9 allocs. The filtering package passed under the race detector, make go-check passed in full, govulncheck found no reachable vulnerabilities, and git diff --check passed. I found no new blocker in the current head.

[Sil3ntVip3r]

Re-reviewed exact head 6e6a68cf43168a4b0e088809e1d44b49fb773b04. The only change since the previously approved head replaces SHA-256 with one process-seeded maphash value. The seed is stable for the life of the process, each fingerprint uses its own hash instance, and the value is only compared with the currently installed engine fingerprint. This preserves the content-identity behavior while making deliberate input collisions impractical; the residual 64-bit accidental-collision risk is negligible for this cache-invalidation use.

I independently compared the unchanged-rebuild benchmark with the SHA-256 parent on macOS ARM64. Across five runs, the 100k case fell from 1.01–1.40 ms/op to 0.33–0.57 ms/op, with allocations effectively unchanged at about 33.3 KiB and one fewer allocation. The 10k case showed the same direction. Absolute values differ from the author measurements, but the reduction is repeatable and supports the stated tradeoff.

Validation on this exact head passed the fingerprint/GC/init-filtering tests under -race repeated ten times, go vet ./internal/filtering, git diff --check, and the complete make go-check gate including the full race suite and govulncheck. I found no new blocker in the maphash change.

[Sil3ntVip3r]

Re-reviewed exact head b3938476bc9a68b3b9c7778fb94d214e14753314.

The new pooled fingerprint buffer is safe in this path: each call borrows a distinct slice from the concurrency-safe pool, onlyReader ensures io.CopyBuffer uses that buffer rather than os.File.WriteTo, and the added large-file regression verifies bytes beyond the first 32 KiB are included in the fingerprint.

Validation completed:

• relevant filtering/GC/init tests under -race, repeated ten times: passed;
• go vet ./internal/filtering and diff checks: passed;
• complete make go-check (full race suite and govulncheck): passed.

Independent five-run benchmark on macOS ARM64 for unchanged_10000:

• parent: 32.86–33.92 µs/op, 33,302–33,317 B/op;
• this head: 31.97–32.13 µs/op, 531–534 B/op.

That confirms the intended garbage reduction with no material latency regression. Approved.