Skip to content

perf: cache special-scheme flag and use a map for scheme lookups#7

Merged
dunglas merged 1 commit into
mainfrom
perf/scheme-lookups
Apr 20, 2026
Merged

perf: cache special-scheme flag and use a map for scheme lookups#7
dunglas merged 1 commit into
mainfrom
perf/scheme-lookups

Conversation

@dunglas
Copy link
Copy Markdown
Owner

@dunglas dunglas commented Apr 20, 2026

Summary

  • Call protocolComponentMatchesSpecialScheme() once per New() and reuse the result for the hostname and pathname branches instead of matching the component regex against the five special schemes twice.
  • Replace the []string specialSchemeList with a map[string]struct{} so processHostnameForInit / processPathnameForInit use O(1) lookups instead of linear scans. protocolComponentMatchesSpecialScheme ranges over the map keys — order is irrelevant since it returns on the first hit.
  • Collapse the port-defaulting loop into a direct DefaultPorts lookup, which already keys the same five schemes.

~5–10% reduction in ns/op on BenchmarkNew across pattern shapes.

Copilot AI review requested due to automatic review settings April 20, 2026 11:08
Copy link
Copy Markdown

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 optimizes URLPattern initialization by reducing repeated special-scheme checks and improving scheme membership lookups, targeting lower allocation/time costs in New().

Changes:

  • Convert the special-scheme collection from a slice to a set-like map[string]struct{} for O(1) membership checks.
  • Cache protocolComponentMatchesSpecialScheme() once per URLPatternInit.New() and reuse it for hostname/pathname handling.
  • Simplify port defaulting by using a direct DefaultPorts lookup.

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

Comment thread urlpattern.go Outdated
Copy link
Copy Markdown

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

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


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

Comment thread urlpattern.go
Comment thread urlpattern.go Outdated
- Call protocolComponentMatchesSpecialScheme() once per New() and reuse
  the result for both the hostname and pathname branches instead of
  matching the component regex against the five special schemes twice.
- Replace specialSchemeList with a single map[string]struct{} (renamed
  to specialSchemeSet since the value is now a set, not an ordered list)
  so the per-component lookups in processHostnameForInit and
  processPathnameForInit are O(1). protocolComponentMatchesSpecialScheme
  just iterates the map keys, since order is irrelevant when the loop
  returns on the first hit.
- Collapse the port-defaulting loop into a direct DefaultPorts lookup
  gated on specialSchemeSet membership, so user-added DefaultPorts
  entries cannot silently trigger special-scheme behaviour. The
  processedInit protocol is lowercased before the check because in
  "pattern" mode processProtocolForInit does not canonicalize, and the
  protocol component is later compiled with canonicalizeProtocol which
  lowercases — so a mixed-case literal like "HTTP" should behave as
  "http" for port defaulting.

~5-10%% reduction in ns/op on New() across pattern benchmarks.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


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

@dunglas dunglas merged commit 8af4f1e into main Apr 20, 2026
8 checks passed
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.

2 participants