From 5566d6c0ab1f92c04ba0dbff3ad45d948c09cf5a Mon Sep 17 00:00:00 2001 From: John Dvorak Date: Sun, 2 Aug 2026 13:22:43 -0700 Subject: [PATCH] perf: binary direct checks skip cycle machinery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _checkBinary ran the visited-set cycle detection (visit-key string allocation + Set has/add) on every call, including top-level direct checks that never recurse — making the 'fast' binary path 2.5x slower than the normal fast path, which already skips the block when _visited is empty. The cycle block now sits after the direct fast path: direct checks return before it, while the rule-evaluation branches (which recurse via evaluateRule with the shared _visited) still detect cycles. Binary direct checks drop from 6.7us to 4.1us avg; the remaining 1.4x is the honest cost of the richer binary result contract (allow/deny, threshold compare, config lookup). Parity suites and full rigor remain green. --- src/authorization/AuthorizationChecker.js | 66 ++++++++++++----------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/src/authorization/AuthorizationChecker.js b/src/authorization/AuthorizationChecker.js index 3644d59..afffad8 100644 --- a/src/authorization/AuthorizationChecker.js +++ b/src/authorization/AuthorizationChecker.js @@ -659,37 +659,6 @@ export class AuthorizationChecker { }; } - // Check for cycles using efficient approach - const useKeyedVisited = this._getVisitedMode(_visited); - const visitKey = useKeyedVisited ? this._getVisitedKey(userId, relation, objectId) : null; - if (useKeyedVisited) { - if (_visited.has(visitKey)) { - return { - possibility: 0, - reliability: 0, - validity: includeMeta ? DEFAULT_VALIDITY : minimalValidity(DEFAULT_VALIDITY), - reason: 'cycle', - binary: true, - ...(evaluation && { evaluation }) - }; - } - } else { - for (const visited of _visited) { - if (visited.userKey === userKey && visited.relation === relation && visited.objectKey === objectKey) { - return { - possibility: 0, - reliability: 0, - validity: includeMeta ? DEFAULT_VALIDITY : minimalValidity(DEFAULT_VALIDITY), - reason: 'cycle', - binary: true, - ...(evaluation && { evaluation }) - }; - } - } - } - - _visited.add(useKeyedVisited ? visitKey : { userKey, relation, objectKey }); - const config = this.arbiter.relationConfigs.get(relation); if (!config) { return { @@ -766,6 +735,41 @@ export class AuthorizationChecker { } } + // Check for cycles using efficient approach. + // Deliberately placed AFTER the direct fast path: direct checks never + // recurse, so they must not pay for key allocation / Set mutation. Only + // the rule-evaluation branches (which recurse via evaluateRule with the + // shared _visited) need cycle detection. + const useKeyedVisited = this._getVisitedMode(_visited); + const visitKey = useKeyedVisited ? this._getVisitedKey(userId, relation, objectId) : null; + if (useKeyedVisited) { + if (_visited.has(visitKey)) { + return { + possibility: 0, + reliability: 0, + validity: includeMeta ? DEFAULT_VALIDITY : minimalValidity(DEFAULT_VALIDITY), + reason: 'cycle', + binary: true, + ...(evaluation && { evaluation }) + }; + } + } else { + for (const visited of _visited) { + if (visited.userKey === userKey && visited.relation === relation && visited.objectKey === objectKey) { + return { + possibility: 0, + reliability: 0, + validity: includeMeta ? DEFAULT_VALIDITY : minimalValidity(DEFAULT_VALIDITY), + reason: 'cycle', + binary: true, + ...(evaluation && { evaluation }) + }; + } + } + } + + _visited.add(useKeyedVisited ? visitKey : { userKey, relation, objectKey }); + // Handle logical operators with binary evaluation if (config.union || config.intersection || config.exclusion) { const res = this.ruleEvaluator.evaluateRule(