Investigate flood of suggestionsDatabaseUpdate messages on startup
#6706
Comments
|
Hubert Plociniczak reports a new STANDUP for yesterday (2023-05-15): Progress: Investigating reported performance issues on #6630. Initial assessment of #6691. #6655 revealed problems with caching on circular dependencies, will need further investigation for a proper fix. It should be finished by 2023-05-18. Next Day: Next day I will be working on the #6706 task. Continue investigating perf issues. |
|
What is the reason batch updates were "abandoned"? If they were abandoned, there has to be reason for it. Instead of fixing this issue now somehow, we should chat about what was the reason and what is the proper solution here. For example, if batch updates were too big to sent via network, flooding GUI with a lot of updates might be a good idea, but we should then implement their batching on our side. Anyway, this needs discussion/proper information before implementation. |
DB updates for libraries suggestions were previously created during the startup. There were a lot of them and they were batched. Now we don't batch them but actually create a single message per library. So that was a typo on my side. The flooding that happens appears to be related to regular updates as things are processed. I'm saying "appears to be" because that's part of the investigation and part of the process. That's why I don't see the reason for "chatting about it" with GUI folks, yet. Maybe it's a problem on LS maybe GUI but unless we are actually given time to investigate I can't tell. I can sense the urgency in your tone but I also don't intend to involve too many folks, too early, for obvious resourcing reasons. |
|
Makes sense, thanks for the info. Regarding chatting – let's do it when we will have enough information, of course. If the problem with GUI is because it is being re-rendered a lot of times after these updates, it is not necessarily problem with the amount of updates – maybe we should just batch incoming updates in GUI and apply then next frame / after few frames. Sending updates as a batch might cause network issues and performance drop there (it might not, batched updates might be sent faster). Anyway, all these aspects need to be taken into consideration when we will have the information after the investigation :) Regarding "urgency" in my tone – that was definitely not intentional. The only thing I wanted to catch early here is that the fix for the issue is not obvious and fixes on GUI side might be needed as much as on the engine side. So before we get to implementation, I'd like to have a sync call between all parts here. |
|
Hubert Plociniczak reports a new STANDUP for yesterday (2023-05-16): Progress: Trying to understand how suggestions are being produced. One thing which stands out is that we seem to update suggestions for the expressions with the same type multiple times. Will try to follow that lead. It should be finished by 2023-05-18. Next Day: Next day I will be working on the #6706 task. Continue investigating perf issues. |
|
Followed the hunch and it appears that we issue multiple suggestions updates even if the return type of expressions does not change. This is apparently especially painful if the graph has errors or panics. For the sake of the argument I turned those off and GUI became very responsive. This sounds vaguely similar to #6639. So far it looks like an issue on LS-side. |
|
Timed.zip |
|
Hubert Plociniczak reports a new STANDUP for yesterday (2023-05-17): Progress: Identified the problem. We now update suggestions iff the type of the expression changes, as it was a while ago. Did some extensive testing and adapting tests. It should be finished by 2023-05-18. Next Day: Next day I will be working on the #6706 task. Finish adapting tests and create a PR. |
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 #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](https://github.com/enso-org/enso/assets/292128/35918841-f84f-4e1c-b1b0-40e45d97e111)
github-project-automation
bot
moved this from 🔧 Implementation
to 🟢 Accepted
in Issues Board
|
Hubert Plociniczak reports a new STANDUP for yesterday (2023-05-18): Progress: Finished adapting tests and created a PR. Quite happy with resulting perf improvements. Started digging into several reported issues. Trying to reproduce #6766, #6763, #6756. Created #6767 for another renaming issue. It should be finished by 2023-05-18. Next Day: Next day I will be working on the #6766 task. Continue trying to reproduce reported bugs. |
Separated the issue from ticket #6630.
On a relatively modest project, on startup, we appear to send hundreds of small messages from language server with suggestions updates.
This is intentional, to some extent, because we abandoned batch updates, butApparently it can put a strain on IDE to handle them, especially within such a short period of time.The text was updated successfully, but these errors were encountered: