Repository navigation
perf(filters): parse a filter value in linear time - #121
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The goal is to find and fix measurable performance problems in the routes DMS modules expose through the data API, without changing the exported surface (ANT-1929, wave 2 of the org-wide perf pass). Any client allowed to call a
listroute with a@Filterfield can stall the whole Node process. Sending a longfilter_<field>=value with no colon does it.parseFilterused the unanchored regex/([^:]+):(.*)/. On a colon-free value the regex retries from every start position and rescans the rest of the value each time, so its cost grows with the square of the value's length.What changed
src/internal/filters.tsnow matches/^:*([^:]+):(.*)/. Anchored, the regex tries one start position, so the match is linear.^:*skips the leading colons the old regex also skipped, so every input parses to the same[value, comparison]pair as before. That held on 2M random strings here, and two independent reviewers found 0 differences across 3.5M and 5.5M generated strings, newlines, U+2028 and lone surrogates included.filter_comparisons.test.tsgains two tests:::is:abcstill reads comparisonisand valueabc. A plain^([^:]+):anchor would break that.Evidence
The evidence level is confirmed in an end-to-end benchmark. This is DoS hardening: normal requests do not get measurably faster.
Workload: the
ajs module testharness (in-memory MongoDB,@antelopejs/apiover HTTP). Alistcontroller has one@Filterfield over 100 rows. Each data point is the median of 9 requests. The before and after builds were interleaved 3 times per side on a shared, noisy server (load average about 12 on 16 cores). Numbers below are the median of the 3 runs, with the range in parentheses.listwith a 15,000-char colon-free filterlistwith an 8,000-char colon-free filterlist, idlelistwhile one client sends 15,000-char filters back to backAll responses were 200. Node's default 16 KB header limit answers 431 above about 16,000 characters, so 15,000 characters is close to the worst a single request reaches today, about 120 ms of blocked event loop. At that cost, about 8 such requests per second keep one core busy. Normal parsing goes from 58 ns to 42 ns per value in a microbenchmark, which does not matter.
Cumulative impact: none on normal traffic. A single cheap request that blocks every other request for about 120 ms is gone.
Readability: the regex gains
^:*. It needs no comment, and the leading-colon test pins down why it is there.Exported surface
API: 1 packages, 26 entries unchanged against origin/main (committed HEAD).
API: skipped .agents/skills/formal-field-access, private.
API: skipped .agents/skills/verify-interface, private.
API: skipped test-helper entry ./dist/.d.ts in .; test helpers need a test runner and are outside both proof halves.
Runtime API: 1 packages, 23 entries unchanged against origin/main (committed HEAD).
Runtime API: skipped test-helper entry ./dist/.js in .; test helpers need a test runner and are outside both proof halves.
No behavioural probe ran.
Suppressions in authored code files (origin/main and committed HEAD). A lint directive counts once per rule it names;
*is a directive naming none.anti-slop/no-unsafe-dictionary-typetypescript/no-unsafe-return@ts-expect-error@ts-ignoreSAFETY:commentspnpm run checks:Checks: 11 passed, 0 failed.