Only send suggestions updates when type changes #6755
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The change adds an additional field to `ExpressionUpdates` messages sent by `ProgramExecutionSupport` to indicate if the type of value (or its method pointer) has changed and therefore would potentially require a suggestions' update. Prior to #3729 that check was done during the instrumentation. However we still want to continue to support "pending expression" functionality therefore `SuggestionsHandler` will use the additional information to filter only the required expression updates. Most of the changes are related to adapting our tests to the new field. Closes #6707.
|
Also appears to help slightly with #6639 |
JaroslavTulach
approved these changes
May 18, 2023
hubertp
commented
May 18, 2023
| @@ -386,6 +386,7 @@ object ProgramExecutionSupport { | |||
| Api.ProfilingInfo.ExecutionTime(e.getNanoTimeElapsed) | |||
| }.toVector, | |||
| value.wasCached(), | |||
| value.isTypeChanged() || value.isFunctionCallChanged(), | |||
There was a problem hiding this comment.
This change + filtering in SuggestionsHandler is the gist of this PR. The rest is just noise in tests.
There was a problem hiding this comment.
Glad that it worked out. One unfortunate thing is that we cannot filter those early in the instrument like it was done originally. But as I understand, we need to propagate them to the language server to handle errors and panics properly.
4e6
approved these changes
May 18, 2023
| @@ -265,6 +265,7 @@ final class SuggestionsHandler( | |||
| updates.map(u => (u.expressionId, u.expressionType)) | |||
| ) | |||
| val types = updates.toSeq | |||
| .filter(_.typeChanged) | |||
Akirathan
approved these changes
May 18, 2023
mergify
bot
deleted the
wip/hubert/6706-frequent-suggestions-on-expression-update
branch
5 tasks
Procrat
added a commit
that referenced
this pull request
May 22, 2023
…t-rename * develop: Widgets, Vector as Column, Viz Fixes and Rename Columns (#6768) Implement simple variants of `parse` for the Database backend (#6731) Enable `require-jsdoc` lint and add two lints related to React (#6403) Decimal/Integer .round and .int #6654 (#6743) Set suggestion reexports when serializing the library (#6778) Fix file uploading node expression. (#6689) Using WarningsLibrary to query for warnings (#6751) Implement `cast` for Table and Column (#6711) Display Initializing project... message when initializing project (#6661) Only send suggestions updates when type changes (#6755) sbt runEngineDistribution --debug to ease debuggging (#6745) Display "modified at" column on the cloud dashboard (#6687) Meta.meta Integer . methods (#6740) Show spinner while loading directory (#6714) Add cloud dashboard to changelog (#6688) Fix list editor panics during insertion (#6540) Update bug-report.yml Remove project create form (#6710) Change full-screen visualisation shortcut to shift-space (#6663)
Procrat
added a commit
that referenced
this pull request
May 22, 2023
* develop: (30 commits) Widgets, Vector as Column, Viz Fixes and Rename Columns (#6768) Implement simple variants of `parse` for the Database backend (#6731) Enable `require-jsdoc` lint and add two lints related to React (#6403) Decimal/Integer .round and .int #6654 (#6743) Set suggestion reexports when serializing the library (#6778) Fix file uploading node expression. (#6689) Using WarningsLibrary to query for warnings (#6751) Implement `cast` for Table and Column (#6711) Display Initializing project... message when initializing project (#6661) Only send suggestions updates when type changes (#6755) sbt runEngineDistribution --debug to ease debuggging (#6745) Display "modified at" column on the cloud dashboard (#6687) Meta.meta Integer . methods (#6740) Show spinner while loading directory (#6714) Add cloud dashboard to changelog (#6688) Fix list editor panics during insertion (#6540) Update bug-report.yml Remove project create form (#6710) Change full-screen visualisation shortcut to shift-space (#6663) Revert "Show spinner when opening/creating a project (#6321)" (#6712) ...
mergify bot
pushed a commit
that referenced
this pull request
May 29, 2023
Only invalidate the graph editor view at most once per frame. On develop, this saves about 70ms (2%). Testing a recent backend without #6755 as a stress-test, this saves about 5s (45%). This reflects better scalability to large numbers of `SuggestionUpdate` messages. Fixes #6630. # Important Notes - Also fix intermittent profiling failures occurring since the introduction of microtasks.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Description
The change adds an additional field to
ExpressionUpdatesmessages sent byProgramExecutionSupportto indicate if the type of value (or its method pointer) has changed and therefore would potentially require a suggestions' update.Prior to #3729 that check was done during the instrumentation. However we still want to continue to support "pending expression" functionality therefore
SuggestionsHandlerwill use the additional information to filter only the required expression updates.Most of the changes are related to adapting our tests to the new field.
Closes #6706.
Important Notes
The associated project now loads and navigates smoothly.
Also attaching a screenshot from the project that illustrates that pending functionality continues to work:
Kazam_screencast_00006.webm
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
[ ] The documentation has been updated, if necessary.Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build.