ZO minor fixes #2628
ZO minor fixes #2628
Conversation
WalkthroughThe pull request introduces enhancements to progress tracking and reporting in worker-related components across multiple files. The changes focus on adding more detailed progress updates during optimization processes, specifically in Changes
Sequence DiagramsequenceDiagram
participant MainThread
participant ParentWorker
participant Workers
MainThread->>ParentWorker: Start optimization
ParentWorker->>ParentWorker: Post initial progress (0/0)
ParentWorker->>Workers: Spawn workers
Workers-->>ParentWorker: Compute builds
ParentWorker->>MainThread: Update progress incrementally
Workers-->>ParentWorker: Complete optimization
ParentWorker->>MainThread: Final progress report
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
[zzz-frontend] [Sun Jan 26 17:40:45 UTC 2025] - Deployed 4004a2b to https://genshin-optimizer-prs.github.io/pr/2628/zzz-frontend (Takes 3-5 minutes after this completes to be available) [sr-frontend] [Sun Jan 26 17:41:02 UTC 2025] - Deployed 4004a2b to https://genshin-optimizer-prs.github.io/pr/2628/sr-frontend (Takes 3-5 minutes after this completes to be available) [Sun Jan 26 17:52:56 UTC 2025] - Deleted deployment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
libs/zzz/solver/src/parentWorker.ts (1)
125-136: LGTM! Consider adding error handling for postMessage.The addition of initial progress updates aligns well with the PR objective to show progress earlier in the build process. The placement is correct, occurring right after worker initialization.
Consider wrapping postMessage calls in a try-catch block to handle potential messaging errors:
+ function safePostMessage(message: ParentMessage) { + try { + postMessage(message) + } catch (error) { + console.error('Failed to post message:', error) + } + } + // post initial progress - postMessage({ + safePostMessage({ resultType: 'progress', progress: { numBuildsKept: 0, numBuildsComputed: 0, }, })libs/sr/solver/src/parentWorker.ts (2)
137-149: LGTM! Consider extracting common worker logic.The addition of initial progress updates is consistent with the changes in zzz/solver and aligns with the PR objectives.
Consider creating a shared worker utility module to reduce code duplication between sr/solver and zzz/solver. This could include:
- Progress tracking logic
- Message posting utilities
- Common worker management code
Example structure:
// libs/common/worker/progress.ts export interface ProgressResult { numBuildsKept: number numBuildsComputed: number } export function initializeProgress(): void { postMessage({ resultType: 'progress', progress: { numBuildsKept: 0, numBuildsComputed: 0, }, }) } // Additional shared worker utilities...
149-149: Implement build filtering optimization.Both sr/solver and zzz/solver have TODO comments about sending the lowest build value to child workers for early filtering. This optimization could significantly improve performance by allowing workers to skip builds that won't make the cut.
Would you like me to help implement this optimization? The solution would involve:
- Tracking the lowest kept build value
- Implementing a new message type for updating the threshold
- Adding filtering logic in child workers
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
libs/sr/solver/src/parentWorker.ts(1 hunks)libs/zzz/page-optimize/src/WorkerSelector.tsx(1 hunks)libs/zzz/solver/src/parentWorker.ts(1 hunks)
Describe your changes
Issue or discord link
Testing/validation
Checklist before requesting a review (leave this PR as draft if any part of this list is not done.)
yarn run mini-cilocally to validate format and lint.Summary by CodeRabbit
New Features
User Interface
Performance