fix: dev-tools only set monaco model and text when changing #2417
Conversation
HollowMan6
changed the title
Context: - mit-cml/workspace-multiselect#62 (comment) - mit-cml/workspace-multiselect#62 (comment) - mit-cml/workspace-multiselect#62 (comment) In multi-select plugin, a significant amount of workspace change events (for each block) can be triggered at the same time, which causes the garbage collection mechanism to fail, so that we eventually have JS heap overflow and the page crashed in Chromium. This commit introduces a workaround for this, so that we only update the page when we have model or text updates in monaco. Signed-off-by: Hollow Man <hollowman@opensuse.org>
There was a problem hiding this comment.
Hi HollowMan6. Interesting situation.
Is there a bug open against Monaco and/or Chrome for this apparent garbage collection failure?
The workaround code seems perfectly reasonable but I would want it to be documented with reference to a bug so that it can be removed when the bug is fixed.
|
Hi @cpcallen! Thanks for taking a look at this PR.
No, I haven't filed a bug about this, and I guess this might not even be qualified as a "bug" on their side, as I saw that we update the editor with the same text almost 30 times at the same time when that garbage collection fails, which is unreasonable.
Actually, I think it might be a good idea to have this workaround code even if the bug is fixed somehow, as we don't need to disturb the editor if the text and model are unchanged (so this might even improve the performance). |
I'd grant you that it is foolish, but the it is the crash which is unreasonable. Either there is:
Especially if this does not occur in Firefox then it seems like it is probably 1 (and certainly not 3), so I think this deserves to be reported upstream. Maybe reporting it to Monaco in the first instance, and let them report it upstream? Nevertheless…
Upon reflection I agree with you about this. |
|
Sure, I will file a bug report to monaco first.
Edit: Monaco asks to reproduce the bug in the playground, but it seems like I can't reproduce it anyway in their playground (only see heap size surges but drops quickly), so it shouldn't be 2 and 3. So I think it's more related to Chrome or even V8 JavaScript engine, about their memory management, when computing performance drops (e.g. with heavy rendering load), the garbage collection fails to start, file a Chrome issue should be a proper choice. |
The basics
The details
Resolves
Fixes:
Proposed Changes
This commit introduces a workaround, so that we only update the page when we have model or text updates in monaco.
Reason for Changes
In multi-select plugin, a significant amount of workspace change events (for each block) can be triggered at the same time, which causes the garbage collection mechanism to fail, so that we eventually have JS heap overflow and the page crashed in Chromium.
Test Coverage
Documentation
Additional Information