Skip to content

perf(trie): remove recursive substring traversal#780

Open
q1000treadz wants to merge 1 commit into
NaturalNode:masterfrom
q1000treadz:perf-trie-remove-substring-recursion
Open

perf(trie): remove recursive substring traversal#780
q1000treadz wants to merge 1 commit into
NaturalNode:masterfrom
q1000treadz:perf-trie-remove-substring-recursion

Conversation

@q1000treadz

Copy link
Copy Markdown

Problem

Trie currently traverses strings recursively and creates a new substring at each character step with substring(1).

This adds unnecessary work in hot paths and makes long inputs depend on JavaScript call stack depth.

Approach

Replace recursive per-character traversal with index-based iteration in:

  • addString
  • contains
  • findPrefix
  • findMatchesOnPath

keysWithPrefix now uses iterative traversal only for locating the prefix node. The recursive subtree walk is intentionally preserved to avoid changing result ordering.

getSize() is intentionally unchanged because it does not create substrings and is not part of the hot string traversal path.

Compatibility

No public API changes.

Preserved behavior:

  • dictionary remains an object created with Object.create(null)
  • repeated addString() still returns whether the word already existed
  • empty string behavior is unchanged
  • case-sensitive and case-insensitive behavior is unchanged
  • findPrefix() return values are unchanged
  • findMatchesOnPath() result order is unchanged
  • keysWithPrefix() result order is unchanged
  • string traversal still uses JavaScript UTF-16 code units

Tests

Added regression tests for:

  • very long string insertion and lookup
  • shared prefixes
  • repeated insertion
  • existing and missing keys
  • case-insensitive mode
  • empty string behavior
  • exact findPrefix() result
  • exact findMatchesOnPath() result and order

Verified locally:

  • npm run build:tests
  • npm run lint

Benchmarks

I compared the previous recursive implementation against the new iterative implementation locally.

The comparison checked:

  • build: inserting all words with addString
  • contains hit: contains() for words that exist in the trie
  • contains miss: contains() for words that do not exist in the trie
  • findPrefix: longest-prefix lookup with findPrefix()
  • compatibility: old vs new outputs for addString, contains, findPrefix, findMatchesOnPath, keysWithPrefix, and getSize

Environment:

  • Node.js: v22.15.1
  • Platform: darwin 23.5.0 arm64
  • Compatibility checks: 593 passed
  • Results are medians over multiple rounds

Real word list

Dataset Build contains hit contains miss findPrefix
1k words 1.75x faster 2.08x faster 1.89x faster 2.00x faster
10k words 1.90x faster 2.42x faster 2.02x faster 2.14x faster
100k words 1.46x faster 2.05x faster 1.71x faster 1.91x faster
416k words 1.59x faster 1.83x faster 1.55x faster 1.79x faster

Synthetic datasets

Dataset Build contains hit contains miss findPrefix
shared-prefix 10k 1.80x faster 2.36x faster 2.21x faster 2.83x faster
shared-prefix 100k 2.07x faster 2.45x faster 2.19x faster 2.82x faster
low-prefix 10k 0.87x 0.92x 0.93x 1.02x faster
low-prefix 100k 1.37x faster 1.01x faster 0.89x 1.09x faster

The main improvement comes from avoiding recursive calls and avoiding intermediate substring allocation at each character step. The change also removes the risk of call stack overflow for very long strings.

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