⚡ perf: avoid unnecessary Date allocations in calculateMostActiveDay - #571
⚡ perf: avoid unnecessary Date allocations in calculateMostActiveDay#571is0692vs wants to merge 2 commits into
Conversation
…n most active day In `calculateMostActiveDay`, `new Date()` was unnecessarily instantiated in a loop simply to extract the day of the week. This commit replaces it with an inline implementation of Sakamoto's algorithm utilizing fast character code parsing from the `YYYY-MM-DD` string, eliminating the allocation bottleneck entirely and dropping evaluation time from ~528ms to ~50ms (for 1,000,000 iterations). Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
👋 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 New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
|
Warning Review limit reached
Next review available in: 100 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const t = [0, 3, 2, 5, 0, 3, 5, 1, 4, 6, 2, 4]; | ||
| let year = y; | ||
| if (m < 3) year -= 1; | ||
| const weekday = (year + Math.floor(year / 4) - Math.floor(year / 100) + Math.floor(year / 400) + t[m - 1] + d) % 7; |
There was a problem hiding this comment.
Date#getUTCDay() を固定位置パーサーと Sakamoto のアルゴリズムへ置き換えていますが、既存テストは mostActiveDay の非 null 値を検証していません。うるう日や年境界を含む代表的な日付で従来と同じ曜日になることをテストしないと、今後の計算式や文字位置の変更による集計ずれを検出できません。
Knowledge Base Used: GitHub Client
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/github.ts
Line: 116
Comment:
**曜日計算の回帰テスト不足**
`Date#getUTCDay()` を固定位置パーサーと Sakamoto のアルゴリズムへ置き換えていますが、既存テストは `mostActiveDay` の非 `null` 値を検証していません。うるう日や年境界を含む代表的な日付で従来と同じ曜日になることをテストしないと、今後の計算式や文字位置の変更による集計ずれを検出できません。
**Knowledge Base Used:** [GitHub Client](https://app.greptile.com/hiroki-org/-/custom-context/knowledge-base/hiroki-org/github-user-summary/-/docs/github-client.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Changed `let m` to `const m` in the fast Sakamoto's weekday calculation to resolve the ESLint `prefer-const` error that was failing CI. Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
1 participant
💡 What: Replaced the
new Date()object instantiation inside thecalculateMostActiveDayloop with a zero-allocation inline implementation of Sakamoto's algorithm that parses standardYYYY-MM-DDstrings directly via character codes.🎯 Why: The GitHub Contribution API provides date strings in a strict
YYYY-MM-DDformat. Instantiating a fullDateobject on every iteration of the calendar loop solely to extract the day of the week creates unnecessary object allocation and garbage collection overhead, dragging down performance during heavy data processing.📊 Measured Improvement:
In a benchmark running 1,000,000 date conversions (10,000 dates repeated 100 times):
PR created automatically by Jules for task 12162389470424925193 started by @is0692vs
Greptile Summary
GitHub の contribution calendar から曜日を算出する処理を、Date オブジェクト生成から固定位置の文字コード解析と Sakamoto のアルゴリズムへ置き換えています。
Confidence Score: 4/5
現在の GitHub API の YYYY-MM-DD 入力では安全にマージできそうですが、新しい曜日計算には非ブロッキングの回帰テスト追加を推奨します。
固定位置パーサーと Sakamoto のアルゴリズムは到達可能な日付形式で従来の UTC 曜日計算と一致しており、確認された懸念は計算結果を直接検証するテストがないことだけです。
Files Needing Attention: src/lib/github.ts
Important Files Changed
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "perf(github): replace Date allocation wi..." | Re-trigger Greptile
Context used: