-
Notifications
You must be signed in to change notification settings - Fork 1
Closed
Description
Category: performance
Severity: patch
File(s): background.js (l.~170)
Description
When initialHostByRequest reaches MAX_TRACKED_REQUESTS, trackInitialHost iterates the map to find a stale (TTL-expired) entry and breaks after finding the first one. This means only one slot is freed per insertion under pressure, so bursts of requests can cause repeated single-entry evictions instead of a bulk cleanup. The dedicated cleanupStaleTrackedRequests interval handles background cleanup, but under sustained load the per-insertion eviction loop is inefficient. Evicting all expired entries at once when the cap is reached would be more robust.
Problematic code example
if (initialHostByRequest.size >= MAX_TRACKED_REQUESTS) {
const now = Date.now();
let evictKey = null;
for (const [key, entry] of initialHostByRequest) {
if (now - entry.trackedAt > REQUEST_TRACK_TTL_MS) {
evictKey = key;
break; // ← stops after the first stale entry
}
}
if (evictKey === null) {
evictKey = initialHostByRequest.keys().next().value;
}
initialHostByRequest.delete(evictKey);
redirectedRequestIds.delete(evictKey);
}Suggested fix
Collect and delete all stale entries in one pass before falling back to the oldest-inserted eviction:
if (initialHostByRequest.size >= MAX_TRACKED_REQUESTS) {
const now = Date.now();
const staleKeys = [];
for (const [key, entry] of initialHostByRequest) {
if (now - entry.trackedAt > REQUEST_TRACK_TTL_MS) {
staleKeys.push(key);
}
}
if (staleKeys.length > 0) {
for (const key of staleKeys) {
initialHostByRequest.delete(key);
redirectedRequestIds.delete(key);
}
} else {
const evictKey = initialHostByRequest.keys().next().value;
initialHostByRequest.delete(evictKey);
redirectedRequestIds.delete(evictKey);
}
}Acceptance criteria
- All TTL-expired entries are removed in a single pass when the cap is reached
- Fallback to oldest-inserted eviction is used only when no stale entry exists
- Both
initialHostByRequestandredirectedRequestIdsare kept in sync - Unit tests cover the bulk-eviction path
Reactions are currently unavailable