Skip to content

experiment with StringBuilder to guard strings under construction#448

Merged
davidhewitt merged 3 commits into
mainfrom
dh/expandtabs
May 29, 2026
Merged

experiment with StringBuilder to guard strings under construction#448
davidhewitt merged 3 commits into
mainfrom
dh/expandtabs

Conversation

@davidhewitt
Copy link
Copy Markdown
Collaborator

@davidhewitt davidhewitt commented May 19, 2026

WIP, pushing to see pipeline feedback.


Summary by cubic

Introduce StringBuilder to enforce resource-tracked string growth and prevent OOMs from unbounded intermediates. Adopt it in string padding and tab expansion so oversized builds fail fast with MemoryError.

  • New Features

    • Added string_builder::StringBuilder that reserves bytes with the ResourceTracker as it grows, and releases on drop/finish; 2× growth reduces tracker calls.
    • Implements fmt::Write; tracker errors surface via finish. Added TODOs in py_repr/py_str to route repr/str building through StringBuilder.
  • Bug Fixes

    • str.expandtabs, center, ljust, rjust, and zfill now build via StringBuilder, with exact-capacity reservations for bounded paths (s.len() + pad) and safe 2× growth for unbounded expandtabs.
    • Added str_expandtabs_memory_limit test; expanded CLAUDE.md with builder guidance.

Written for commit 6aeb9cd. Summary will update on new commits.

Review in cubic

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 19, 2026

Merging this PR will not alter performance

✅ 17 untouched benchmarks
⏩ 15 skipped benchmarks1


Comparing dh/expandtabs (6aeb9cd) with main (fbdb2e1)

Open in CodSpeed

Footnotes

  1. 15 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 19, 2026

Codecov Results 📊

❌ Patch coverage is 0.93%. Project has 24223 uncovered lines.
❌ Project coverage is 66.1%. Comparing base (base) to head (head).

Files with missing lines (3)
File Patch % Lines
crates/monty/src/string_builder.rs 0.00% ⚠️ 73 Missing
crates/monty/src/types/str.rs 0.00% ⚠️ 33 Missing
crates/monty/src/types/py_trait.rs 50.00% ⚠️ 1 Missing
Coverage diff
@@            Coverage Diff             @@
##          main       #PR       +/-##
==========================================
- Coverage    66.19%    66.10%    -0.09%
==========================================
  Files          279       281        +2
  Lines        71052     71447      +395
  Branches    152853    153474      +621
==========================================
+ Hits         47030     47224      +194
- Misses       24022     24223      +201
- Partials      3993      4044       +51

Generated by Codecov Action

@codecov
Copy link
Copy Markdown

codecov Bot commented May 19, 2026

Codecov Report

❌ Patch coverage is 72.22222% with 30 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/monty/src/string_builder.rs 69.86% 19 Missing and 3 partials ⚠️
crates/monty/src/types/str.rs 78.78% 3 Missing and 4 partials ⚠️
crates/monty/src/types/py_trait.rs 50.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 5 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread crates/monty/src/types/str.rs Outdated
Comment thread crates/monty/src/types/str.rs
@davidhewitt davidhewitt enabled auto-merge (squash) May 29, 2026 19:08
@davidhewitt davidhewitt merged commit 7f7d08e into main May 29, 2026
30 checks passed
@davidhewitt davidhewitt deleted the dh/expandtabs branch May 29, 2026 19:14
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