Skip to content

perf: avoid padStart in hot date formatters - #581

Closed
seonghobae wants to merge 2 commits into
developfrom
bolt/optimize-date-padding-16190668027941878146
Closed

perf: avoid padStart in hot date formatters#581
seonghobae wants to merge 2 commits into
developfrom
bolt/optimize-date-padding-16190668027941878146

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

What

Replace String.padStart(2, '0') in formatDateInput, formatLocalDateInput, and formatCompactDate with fixed-width numeric branching while preserving the existing YYYY-MM-DD / YYYYMMDD outputs.

The branch also adds tests/unit/date-formatting-performance.test.mjs and registers it in both test:unit and test:coverage:cases. The regression deliberately makes String.prototype.padStart throw while exercising the three formatters, so the protected develop implementation is RED and this branch is GREEN for the optimization invariant. It checks both one-digit and two-digit month/day boundaries for UTC and local formatting.

Evidence and scope

A local Node/V8 microbenchmark over the formatter-sized operation (20,000,000 iterations, five rounds) observed lower runtime for the branch form than String.padStart in that runtime. That observation is runtime/JIT-specific; this PR does not claim a universal JS↔C++ transition mechanism, guaranteed GC reduction, or a buyer-visible browser latency improvement without target-browser measurement.

.jules/bolt.md is narrowed accordingly: keep the deterministic no-padStart semantic regression for this path, and re-measure in the target runtime before generalizing the optimization elsewhere.

CI authority

The previous hosted runs were green but checked GitHub's synthetic PR merge ref rather than the exact contributor head. They are therefore non-authoritative for exact-head acceptance under the repository's current quality contract. PR #523 contains the ScopeWeave-owned exact-head CI repair; until that control lands, this PR must not be merged based on merge-ref success alone.

No qualifying independent current-head approval is present. Do not count author/model-only comments as approval.

Summary by CodeRabbit

  • 개선 사항

    • 날짜 표시 형식을 기존과 동일하게 유지하면서 월·일 포맷팅 성능을 개선했습니다.
    • 한 자리 월·일도 안정적으로 두 자리 형식으로 표시됩니다.
    • UTC 및 로컬 날짜 모두 YYYY-MM-DD, YYYYMMDD 형식으로 일관되게 출력됩니다.
  • 품질 개선

    • 다양한 날짜 입력에 대한 형식과 성능 검증을 강화했습니다.

- `app.js` 내부의 빈번하게 호출되는 날짜 변환 핫 루프(`formatDateInput`, `formatLocalDateInput`, `formatCompactDate`)에서 사용되던 `String.padStart()`를 제거했습니다.
- 대신 인라인 삼항 연산자를 이용한 문자열 병합을 도입하여 불필요한 객체 할당과 JS-C++ 브릿지 통신 오버헤드를 감소시켰습니다.
- 관련 학습 내용을 한국어로 `.jules/bolt.md`에 기록했습니다.
@google-labs-jules

Copy link
Copy Markdown

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2d6043e3-03bc-4e53-89e0-528af2597c79

📥 Commits

Reviewing files that changed from the base of the PR and between 01f79ac and b272f04.

📒 Files selected for processing (3)
  • .jules/bolt.md
  • package.json
  • tests/unit/date-formatting-performance.test.mjs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

세 날짜 포맷팅 함수가 padStart 대신 조건부 접두사 방식으로 월과 일을 두 자리로 변환합니다. 관련 단위 테스트와 커버리지 실행 항목을 추가하고, 최적화 지침을 측정 결과 기반으로 수정합니다.

Changes

날짜 포맷팅 최적화

Layer / File(s) Summary
날짜 zero-padding 구현 및 검증
.jules/bolt.md, app.js, tests/unit/date-formatting-performance.test.mjs, package.json
formatDateInput, formatLocalDateInput, formatCompactDate가 한 자리 월과 일 앞에 '0'을 직접 추가합니다. 테스트는 padStart 비사용과 UTC·로컬 날짜의 출력 형식을 검증합니다. 단위 테스트 및 커버리지 실행 목록에 해당 테스트를 추가합니다. 최적화 지침은 Node/V8 측정 결과와 재측정 절차를 반영합니다.

Estimated code review effort: 2 (간단) | ~10 minutes

Merge Risk: ⚪ Minimal · up to b272f

This is a localized date-formatting optimization with regression coverage; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (2 skipped: 2 unsupported.) Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 날짜 포맷터의 핫 패스에서 padStart를 제거하는 주요 변경 사항을 정확하고 간결하게 설명합니다.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bolt/optimize-date-padding-16190668027941878146

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Add a deterministic regression that fails if the three hot-path date formatters return to String.prototype.padStart while checking zero-padded UTC/local outputs. Register it in unit and coverage cases, and narrow the optimization note to measured runtime-specific evidence rather than an unsupported engine-boundary claim.
@seonghobae seonghobae changed the title ⚡ Bolt: 날짜 포맷팅 함수의 패딩 성능 최적화 perf: avoid padStart in hot date formatters Aug 22, 2026
@opencode-agent opencode-agent Bot added priority: medium Normal-priority or P2 work status: needs-review Open pull request requiring current-head review or checks type: maintenance Maintenance, build, dependency, or operational upkeep labels Aug 22, 2026
@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown

Caution

Review failed

An error occurred during the review process. Please try again later.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown
Contributor Author

Closing this performance lane as non-defensible under the current buyer-visible evidence contract rather than merging an implementation-detail micro-optimization without target-runtime impact proof.

Fresh exact state is unchanged at b272f045814f9da981be1793478a062e3ca20d76 against protected develop@2c328875e00e86537df3e965170be80532571cad, Ready/mergeable, with one COMMENTED model review and six terminal-success workflows. Those green workflow labels are not exact-contributor-head merge authority until #523/central checkout controls land.

The production delta replaces three String(...).padStart(2, '0') calls with branch/concatenation. The new unit test is deterministic, but it proves only that String.prototype.padStart is not called while selected date outputs remain equal; monkey-patching padStart to throw makes the implementation choice itself the acceptance criterion. It does not prove a ScopeWeave latency, throughput, memory, rendering, or interaction requirement. The PR body correctly acknowledges that the only timing evidence is a local Node/V8 microbenchmark and makes no buyer-visible browser-latency claim.

That leaves the same commercial-evidence gap already used to reject #499's equivalent date-padding rewrite: no production-browser before/after benchmark tied to a realistic plan workload, no demonstrated material contribution of these three formatter calls to a user-visible hot path, and no acceptance threshold that distinguishes meaningful performance from runtime/JIT noise. The stronger performance lanes (#495/#409 for rendering invalidation and #508 for computeTaskMetrics) bind optimization to realistic browser workloads, deterministic semantic equivalence, and explicit thresholds; this branch does not add unique product value at that level.

No source is being transplanted. If profiling later shows these formatters materially contribute to an actual buyer-visible budget, reintroduce the smallest change inside that measured performance lane with target-browser evidence and a semantic-equivalence regression rather than a no-padStart implementation lock.

@seonghobae seonghobae closed this Aug 22, 2026
@google-labs-jules

Copy link
Copy Markdown

Closing this performance lane as non-defensible under the current buyer-visible evidence contract rather than merging an implementation-detail micro-optimization without target-runtime impact proof.

Fresh exact state is unchanged at b272f045814f9da981be1793478a062e3ca20d76 against protected develop@2c328875e00e86537df3e965170be80532571cad, Ready/mergeable, with one COMMENTED model review and six terminal-success workflows. Those green workflow labels are not exact-contributor-head merge authority until #523/central checkout controls land.

The production delta replaces three String(...).padStart(2, '0') calls with branch/concatenation. The new unit test is deterministic, but it proves only that String.prototype.padStart is not called while selected date outputs remain equal; monkey-patching padStart to throw makes the implementation choice itself the acceptance criterion. It does not prove a ScopeWeave latency, throughput, memory, rendering, or interaction requirement. The PR body correctly acknowledges that the only timing evidence is a local Node/V8 microbenchmark and makes no buyer-visible browser-latency claim.

That leaves the same commercial-evidence gap already used to reject #499's equivalent date-padding rewrite: no production-browser before/after benchmark tied to a realistic plan workload, no demonstrated material contribution of these three formatter calls to a user-visible hot path, and no acceptance threshold that distinguishes meaningful performance from runtime/JIT noise. The stronger performance lanes (#495/#409 for rendering invalidation and #508 for computeTaskMetrics) bind optimization to realistic browser workloads, deterministic semantic equivalence, and explicit thresholds; this branch does not add unique product value at that level.

No source is being transplanted. If profiling later shows these formatters materially contribute to an actual buyer-visible budget, reintroduce the smallest change inside that measured performance lane with target-browser evidence and a semantic-equivalence regression rather than a no-padStart implementation lock.

알겠습니다. 타겟 런타임 성능 입증 증거 부족 및 마이크로 최적화라는 사유로 해당 PR이 종료되었음을 인지했습니다. 본 작업에 대한 진행을 중단하겠습니다. (Understood. Acknowledging that this work is closed as a non-defensible micro-optimization without target-runtime impact proof, and stopping work on this task.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: medium Normal-priority or P2 work status: needs-review Open pull request requiring current-head review or checks type: maintenance Maintenance, build, dependency, or operational upkeep

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant