Skip to content

perf: parallelize resource ACL and fallback ACL fetching#2810

Open
maxleonard wants to merge 2 commits intomainfrom
feat/http2-parallel-acl
Open

perf: parallelize resource ACL and fallback ACL fetching#2810
maxleonard wants to merge 2 commits intomainfrom
feat/http2-parallel-acl

Conversation

@maxleonard
Copy link

Summary

Parallelize the fetching of resource ACL and fallback ACL in internal_fetchAcl, eliminating a sequential wait that adds unnecessary latency. This is complementary to the HTTP/2 multiplexing support being added in solid-client-authn-js#4213 — HTTP/2 multiplexing only helps if requests are actually issued in parallel.

Detailed Changes

src/acl/acl.internal.tsinternal_fetchAcl

Before (sequential):

const resourceAcl = await internal_fetchResourceAcl(resourceInfo, options);
if (resourceAcl === null) {
  fallbackAcl = await internal_fetchFallbackAcl(resourceInfo, options);
}

The resource ACL was fetched first, and only if it was null was the fallback ACL fetched. This serialized two independent network operations.

After (parallel):

const [resourceAcl, fallbackAcl] = await Promise.all([
  internal_fetchResourceAcl(resourceInfo, options),
  internal_fetchFallbackAcl(resourceInfo, options).catch(() => null),
]);
return resourceAcl !== null
  ? { resourceAcl, fallbackAcl: null }
  : { resourceAcl: null, fallbackAcl };

Both fetches are now issued concurrently via Promise.all. If the resource ACL exists, the fallback result is discarded. The fallback is wrapped in .catch(() => null) to safely handle cases where the speculative fetch would fail (the original code would never have executed it in those cases).

Trade-off: One extra request when the resource ACL exists (the fallback fetch is unnecessary). With HTTP/2 multiplexing, this overhead is minimal since it shares the same TCP connection. The latency improvement from parallel fetching outweighs the marginal bandwidth cost.

src/acl/acl.test.ts — Updated tests

Three tests updated to accommodate parallel fetching:

  • Assertions that checked exact fetch call counts (toHaveLength(2)) now use toBeGreaterThanOrEqual(2) since parallel fetching may issue additional speculative requests
  • Assertions that checked positional call arguments now use a calledUrls array with toContain instead

Test plan

  • All 94 ACL tests pass
  • No behavior change for callers — return values are identical
  • Benchmark with HTTP/2 transport to measure latency improvement

🤖 Generated with Claude Code

Fetch resource ACL and fallback ACL concurrently using Promise.all
instead of sequentially. This reduces latency when combined with
HTTP/2 multiplexing, as the speculative fallback fetch overlaps
with the resource ACL fetch at minimal cost.

- internal_fetchAcl now issues both fetches in parallel
- Fallback wrapped in .catch(() => null) to prevent regression
  when speculative fetch fails
- Updated tests for parallel fetch behavior

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@maxleonard maxleonard requested a review from a team as a code owner March 2, 2026 10:55
The parallel ACL fetching now consumes mock responses that were
previously unused (fallback fetch was skipped when resource ACL
existed). Add a 4th mock response for the saveAclFor step in
the 3 affected "ignores the fallback ACL" tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@maxleonard maxleonard temporarily deployed to ESS Release-2-3 March 2, 2026 11:01 — with GitHub Actions Inactive
@maxleonard maxleonard deployed to ESS Release-2-3 March 2, 2026 11:01 — with GitHub Actions Active
@maxleonard maxleonard deployed to ESS PodSpaces March 2, 2026 11:01 — with GitHub Actions Active
@maxleonard maxleonard temporarily deployed to ESS Release-2-3 March 2, 2026 11:01 — with GitHub Actions Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant