Skip to content
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

feat(browser-repl): add virtualisation in shell output COMPASS-7567 #2329

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

mabaasit
Copy link
Contributor

In this PR, I am rendering the ShellOutput in a virtual list, where shell input (editor/password prompt - ShellInputPrompt) is rendered outside of it. Initially I made it (shell input) as part of the list. However with that change, so many other things needed to be taken care of - like persisting password prompt value, getting focus back to it as its visible. With that, currently we always render the ShellInputPrompt at the top and as data piles up, its pushed down. Once its at the bottom, it continues to stay there and does not scroll away as is currently happening in embedded shell.

Its still in draft for two reasons:

  1. Need to confirm with the design team regarding the change in behaviour, where ShellInputPrompt sticks at the bottom (similar to Firefox console)
  2. For comparatively larger documents, CM takes its time to render & parse the text, which creates flicker as we are trying to scroll to the end of the output as something new is rendered. Also, when user scrolls, we are calculating the height of the parent container dynamically (just that we position ShellInputPrompt correctly) and as elements are added/removed, this also creates flicker at time.

I have been playing with few options regarding this. I tested virtualisation using Codemirror (CM) editor, LG Code component (LG).

Currently we are using CM to render the shell input/output. The setup for this component is expensive in terms of time and memory and as a result user sees CPU spike. The spike is unavoidable if we are rendering especially a large document, where CM needs to parse it for syntax highlighting (which it does only when needed). Except for table outputs in shell (show dbs/collection etc), we mostly render everything using this component - even the scalers.

With virtualising the output using CM, the rendering is pretty fast, however we continue to render a single shell output using one CM instance - which brings in CPU spikes and rendering lags.

@gribnoysup suggested that we explore using LG instead. Its slightly better in terms of performance and rendering but only for smaller documents. For collections with higher document size, CM still performs pretty well (as it has inbuilt virtualisation).

Comparison of performance (time) between CM and LG. In this table, only five docs were queried and everytime shell output was cleared.

Average Document Size CM (non-virtual) LG (non-virtual) CM (virtual) LG (virtual)
.5 MB ~ 1.00 s ~ 1.65 s ~ 0.40 s ~ 1.80 s
1 MB ~ 1.70 s ~ 5.00 s ~ 0.67 s
2 MB ~ 2.90 s ~ 5.50 s ~ 1.40 s
3 MB ~ 4.10 s ~ 6.00 s ~ 1.65 s

(I gave up on LG as it started with slow performance for bigger docs).


I feel we should only use one CM instance to render the shell output where we club all the various types of outputs. Currently we have few special use cases (shell commands) to render:

  • Table View - used in help, list dbs, list collections
  • Banner view - where we are currently using h3 for heading.
  • Stats output - combo of hr, h4.
  • Error View

Among these, Error view is the one where we need to do some special handling as it contains a toggle. This all should be possible if we do a custom grammar for CM and then with

For passwordPrompt, we render this component at the shell input and this will remain unaffected if we use single CM for output.

We also have a special input for password, but we are not rendering it in the shell output list, but rather its part of shell input.

@mabaasit mabaasit changed the title feat(browser-repl): add virtualisation in shell output feat(browser-repl): add virtualisation in shell output COMPASS-7567 Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant