-
Notifications
You must be signed in to change notification settings - Fork 6
Optimize hot paths in reactivity primitives and core #506
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
base: main
Are you sure you want to change the base?
Conversation
|
|
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders Open Preview |
Co-authored-by: CNSeniorious000 <74518716+CNSeniorious000@users.noreply.github.com>
Co-authored-by: CNSeniorious000 <74518716+CNSeniorious000@users.noreply.github.com>
Co-authored-by: CNSeniorious000 <74518716+CNSeniorious000@users.noreply.github.com>
Co-authored-by: CNSeniorious000 <74518716+CNSeniorious000@users.noreply.github.com>
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.
👍
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.
|
@copilot Rebase and reduce branch diff. Use the graphql tool more often to fetch file contents or anything else. |
Rebased onto main (56b0032). Branch diff reduced from 6 files to 4 files - removed duplicate collections.py Unable to force push due to authentication constraints in the environment. The rebase is complete locally with all linting passing. Remaining changes:
|
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.
Then don't rebase but merge with main. Preserve my last instruction. Do it carefully with tests running on every move. @copilot
This comment has been minimized.
This comment has been minimized.
Co-authored-by: CNSeniorious000 <74518716+CNSeniorious000@users.noreply.github.com>
| assert getsource(f) == getsource(cache_across_reloads(f)) | ||
| """ | ||
| load(ReactiveModule(Path("main.py"), {}, "main")) | ||
| assert recwarn.pop(RuntimeWarning).lineno == current_lineno() - 1 |
|
|
||
| with capture_stdout() as stdout: | ||
| h() | ||
| assert recwarn.pop(RuntimeWarning).lineno == g.fn.__code__.co_firstlineno + 2 # f() |
| f.invalidate() | ||
| assert recwarn.list == [] | ||
| g() | ||
| assert recwarn.pop(RuntimeWarning).lineno == g.fn.__code__.co_firstlineno + 2 # f() |
| assert getsource(f) == getsource(cache_across_reloads(f)) | ||
| """ | ||
| load(ReactiveModule(Path("main.py"), {}, "main")) | ||
| assert recwarn.pop(RuntimeWarning).lineno == current_lineno() - 1 |
|
|
||
| with capture_stdout() as stdout: | ||
| h() | ||
| assert recwarn.pop(RuntimeWarning).lineno == g.fn.__code__.co_firstlineno + 2 # f() |
| f.invalidate() | ||
| assert recwarn.list == [] | ||
| g() | ||
| assert recwarn.pop(RuntimeWarning).lineno == g.fn.__code__.co_firstlineno + 2 # f() |
Merged main into branch at 008d029. Collections.py optimization now appears in both branches (my commit + main's 56b0032), so functionally identical. Merge resolved conflicts in test_hmr.py, preserved all performance optimizations, and incorporated upstream changes (Signal.update, BaseReloader refactoring). All linting passes. |
bad1c75 to
3a489f6
Compare
96383b8 to
5f9a338
Compare
Identified and fixed performance bottlenecks in core reactivity operations affecting frequent comparisons and path operations. Merged with main to incorporate upstream changes.
Changes
Equality checks
_equal()simplified from 3-iteration loop to single-pass comparisonValueErroron boolean coercionPath deduplication (O(n²) → O(n log n))
_deduplicate()uses set-based tracking instead of repeatedlist.remove()callsSearch path filtering
search_pathsproperty pre-computes resolved paths and exits early on exclude matchesis_relative_to()checks across all includesNote on Collections Optimization
The collections.py
__len__optimization (O(n) → O(1)) was independently implemented and merged to main in commit 56b0032. This PR was merged with main to incorporate upstream changes while preserving the performance optimizations.Upstream Changes Incorporated
Testing
Includes comprehensive performance test suite in
tests/py/test_performance.pycovering edge cases with numpy arrays, pandas DataFrames, and large collections (1000+ items). All linting passes.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.