-
Notifications
You must be signed in to change notification settings - Fork 6
Moving Cell Execution State to Document Awareness #146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
3coins
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Zsailer
This is a great improvement, thanks for working on these changes. Left some minor feedback.
|
Thanks for the feedback @3coins! I've addressed all your suggestions:
The changes improve code readability and maintainability while keeping the same functionality. All updates have been pushed to the branch. |
aa7eff6 to
9a08d76
Compare
…method - Combine three similar else blocks in _updatePrompt method into one - Use optional chaining to simplify null checking in _getCellExecutionStateFromAwareness - Add set_cell_awareness_state method to YRoom for better encapsulation - Update kernel client to use new YRoom method for setting cell states
3coins
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Summary
This PR optimizes cell execution state tracking by moving execution states from YDoc transactions to the document awareness layer, significantly reducing memory usage and improving performance in collaborative Jupyter notebooks.
Key Changes
Architectural Improvements
Performance Optimizations
Technical Implementation
YRoom._cell_execution_states(in-memory)