Skip to content

Fix O(n²) complexity for String.prototype.slice() on rope strings#154

Open
robobun wants to merge 1 commit intooven-sh:mainfrom
robobun:claude/fix-rope-string-slice-performance
Open

Fix O(n²) complexity for String.prototype.slice() on rope strings#154
robobun wants to merge 1 commit intooven-sh:mainfrom
robobun:claude/fix-rope-string-slice-performance

Conversation

@robobun
Copy link

@robobun robobun commented Feb 2, 2026

Summary

When slicing a rope string across fiber boundaries, the previous implementation would resolve the entire rope string, causing O(n) complexity per slice operation. This leads to O(n²) overall complexity when iterating through a concatenated string with slice operations.

Changes

The fix modifies tryJSSubstringImpl in JSString.h to handle cross-fiber slices by extracting substrings from each relevant fiber and concatenating them into a new rope, avoiding the need to resolve the entire original rope.

Performance Impact

Before this fix (50k iterations of slice on a 50k character rope string):

  • Bun: ~2800ms
  • Node.js: ~2ms

After this fix, Bun should match Node.js performance.

Reproduction

let s = '';
for (let i = 0; i < 50000; i++) s += 'A';

let start = Date.now();
let m = new Map();
for (let i = 0; i < 50000; i++) {
    let k = s.slice(i, i+1);
    m.set(k, 1);
}
console.log(`Time: ${Date.now() - start}ms`);

Fixes oven-sh/bun#26682

When slicing a rope string across fiber boundaries, the previous
implementation would resolve the entire rope string, causing O(n)
complexity per slice operation. This leads to O(n²) overall complexity
when iterating through a concatenated string with slice operations.

The fix handles cross-fiber slices by extracting substrings from
each relevant fiber and concatenating them into a new rope, avoiding
the need to resolve the entire original rope.

Before this fix:
- Bun: ~2800ms for 50k iterations
- Node.js: ~2ms for 50k iterations

After this fix, Bun should match Node.js performance.

Fixes oven-sh/bun#26682
robobun pushed a commit to oven-sh/bun that referenced this pull request Feb 2, 2026
String.prototype.slice() currently has O(n) complexity on rope strings,
causing O(n²) overall complexity when iterating through a concatenated
string with slice operations.

The tests are marked as todo until the WebKit fix is merged:
oven-sh/WebKit#154

Once the WebKit PR is merged and the commit hash is updated in
cmake/tools/SetupWebKit.cmake, these tests should be enabled.

Fixes #26682

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 2, 2026

Walkthrough

Reworked tryJSSubstringImpl to extract substrings crossing multiple rope fibers by slicing each involved fiber and constructing new ropes from the resulting parts, replacing previous NULL-return fallback behavior.

Changes

Cohort / File(s) Summary
Rope Substring Extraction
Source/JavaScriptCore/runtime/JSString.h
Enhanced tryJSSubstringImpl to handle multi-fiber substring extraction without full rope resolution. When substrings span fiber0 to fiber1 (and optionally fiber2), the function now slices each fiber and constructs a new rope from parts instead of returning NULL. Added logic for two-fiber and three-fiber crossing scenarios with offset adjustments.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix O(n²) complexity for String.prototype.slice() on rope strings' directly and clearly describes the main change—optimizing string slicing performance by handling cross-fiber rope strings efficiently.
Description check ✅ Passed The description is well-structured with Summary, Changes, Performance Impact, and Reproduction sections explaining the fix clearly, though it does not follow the WebKit template format with Bugzilla link and commit message structure.
Linked Issues check ✅ Passed The PR successfully addresses issue #26682 by implementing multi-fiber substring extraction to avoid resolving entire ropes, transforming O(n²) slice operations into linear time operations.
Out of Scope Changes check ✅ Passed All changes in JSString.h are focused on the tryJSSubstringImpl function to handle cross-fiber rope slices, directly addressing the performance issue identified in #26682 with no extraneous modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

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.

Bad performance of string.slice() on concatenated strings

1 participant