Repository navigation
perf(ai): parse the rule words only on a dictionary cache miss (#7829) - #7850
Conversation
Aias00
left a comment
Aias00
left a comment
There was a problem hiding this comment.
Approved. Moving parseWords inside the cache-miss branch is the right fix and the gain is real: the cache-hit path runs on every request through the sensitive-word plugin, so an O(n²) parse of the rule words was being paid per request to produce a list that the hit path never reads.
I confirmed the variable is only consumed by the miss path — redisAutomaton(...) and the onErrorResume fallback that uses words are both inside that branch, so narrowing the scope does not orphan a reference.
The two tests are well aimed:
testCacheHitDoesNotParseTheRuleWordswarms the dictionary, spies the plugin, and assertsparseWordsis never called. Note this works precisely because the dictionary cache is process-wide, so the warmed entry is visible to the spy — that is the right thing to lean on rather than mocking it away.testCacheMissStillParsesTheRuleWordscovers the opposite direction, and importantly asserts the request is still blocked by the rule word (verify(chain, never()).execute(...)), so a regression where parsing is skipped and the word silently stops being enforced cannot pass.
Two notes, neither blocking:
-
Merge order. This is stacked on #7844, so as long as #7850 lands after it the diff collapses to the one-line move. #7844 is already approved and green, so sequencing is enough — no need to rebuild the branch.
-
parseWordsvisibility. It went fromprivateto package-private purely so the spy can verify it. That is the same trade already made forredisAutomaton/ruleWordsAutomaton; if you want to keep the surface tighter,@VisibleForTestingon it would document the intent.
CI is still running on this branch — the retry-helper change from #7781 may need a re-run, but the affected module tests are green locally per the PR body.
Fixes #7829.
Summary
SensitiveWordPlugin#dictionaryparses the rule words (split+containsdedup, O(n²)) before consulting the dictionary cache. On a cache hit the parsed words are never used, so every hit repeats the parse for nothing.Modifications
parseWordscall into the cache-miss branch, the only place the rule words are consumed (redis merge and the redis-failure fallback).parseWordsto package-private so a spy can pin the parse contract in tests.Verifying this change
testCacheHitDoesNotParseTheRuleWords: warm the cache, then assert the second request never invokesparseWords. Fails on the previous code (NeverWantedButInvoked).testCacheMissStillParsesTheRuleWords: a miss parses exactly once and the rule words are still enforced.Targeted tests pass:
./mvnw -B -ntp -pl shenyu-plugin/shenyu-plugin-ai/shenyu-plugin-ai-sensitive-word test -Dtest=SensitiveWordPluginTestFull
SensitiveWordPluginTest20/20 green.Checkstyle passes:
clean.
Notes
Miss/expiry/failure/fallback paths are unchanged; rule updates already evict the cache entry (
SensitiveWordPluginDataHandler#removeHandle), so a rebuild re-parses fresh words. No measurable-gain claim beyond the removed work — the parse cost scales with the rule word count and vanishes entirely on hits.