Skip to content

Revert "perf(alias): short-circuit load_alias with a 1+2 byte prefix index"#230

Merged
stormslowly merged 1 commit into
mainfrom
revert-225-perf/alias-first-byte-index
May 21, 2026
Merged

Revert "perf(alias): short-circuit load_alias with a 1+2 byte prefix index"#230
stormslowly merged 1 commit into
mainfrom
revert-225-perf/alias-first-byte-index

Conversation

@stormslowly
Copy link
Copy Markdown
Collaborator

@stormslowly stormslowly commented May 21, 2026

Reverts #225

I prefer a trie similar match other than this.

Copilot AI review requested due to automatic review settings May 21, 2026 03:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reverts the previously introduced alias “prefix-byte accelerator” optimization (#225), restoring the prior alias matching behavior in load_alias.

Changes:

  • Removes the AliasFirstBytes precomputed prefix index and the associated fast-path short-circuiting in ResolverGeneric::load_alias.
  • Simplifies ResolverGeneric construction by dropping the now-unneeded alias/fallback prefix index fields.
  • Removes the regression test that covered empty alias keys matching slash-prefixed specifiers.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/lib.rs Reverts the alias prefix-byte accelerator and restores the original load_alias signature/logic.
src/tests/alias.rs Removes the regression test that was added for the reverted optimization.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/tests/alias.rs
@@ -333,25 +333,3 @@ async fn alias_try_fragment_as_path() {
let resolution = resolver.resolve(&f, "#/a").await.map(|r| r.full_path());
assert_eq!(resolution, Ok(f.join("#").join("a.js")));
}
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 21, 2026

Merging this PR will degrade performance by 5.84%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
❌ 4 regressed benchmarks
✅ 7 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Memory resolver[pnp resolve] 8.8 KB 8.4 KB +4.74%
Simulation resolver[pnp resolve] 255.5 µs 264.9 µs -3.55%
Simulation resolver[single-thread] 50.2 ms 52.2 ms -3.84%
Simulation resolver[resolve from symlinks] 144.6 ms 160.4 ms -9.86%
Simulation resolver[resolve from symlinks multi thread] 82.3 ms 97.4 ms -15.48%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing revert-225-perf/alias-first-byte-index (f7472d5) with main (e836a64)

Open in CodSpeed

@stormslowly stormslowly merged commit c8af902 into main May 21, 2026
21 of 22 checks passed
@stormslowly stormslowly deleted the revert-225-perf/alias-first-byte-index branch May 21, 2026 04:40
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.

3 participants