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

[CLOSED] [Performance] Improvements for QuickView #4531

Open
core-ai-bot opened this issue Aug 29, 2021 · 5 comments
Open

[CLOSED] [Performance] Improvements for QuickView #4531

core-ai-bot opened this issue Aug 29, 2021 · 5 comments

Comments

@core-ai-bot
Copy link
Member

Issue by redmunds
Friday Aug 23, 2013 at 20:44 GMT
Originally opened as adobe/brackets#4910


These suggested improvement were split off of #4885.

  1. Current, line is scanned every time mouse moves more than 1 char. Then, delay 350ms before displaying popover. Seems like we could save some (a lot of?) processing by delaying first, then if mouse is still at same position, scan & display. Note that@peterflynn thinks there may have been a good reason it was done this way, so there may be some gotchas.
  2. We check char position to make sure it's in code to popover, but we don't bail out of the loop after we get past current position.
  3. Do not scan lines that are over a certain length. CodeMirror does this for code coloring and matching braces due to bad performance of minified files. I think a length of 1024 is a reasonable place to start.
@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Aug 23, 2013 at 21:22 GMT


This morning, I think we came to the conclusion that the reason for (1) was that it used to start preloading image previews in parallel with the hover delay. However, at some point the code changed and it no longer does that. So there may no longer be any compelling reason to do work on every mousemove.

Re (3), whether line length is a factor is really dependent on the provider. The imagePreviewProvider() only looks at the token the mouse is over, so it will work fine at any line length. So I think we can just put a short-circuit specifically in colorAndGradientPreviewProvider().

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Thursday Aug 29, 2013 at 15:08 GMT


Nominating for Sprint 31.

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Friday Sep 06, 2013 at 20:10 GMT


@redmunds, now that #5040 is merged, do you want to keep this open to address item #3 in the description?

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Friday Sep 06, 2013 at 20:13 GMT


Implementing item 3 will cause a reduction in functionality, so that should only be implemented if items 1 and 2 are not sufficient for all use cases. I think we can close this.

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Friday Sep 06, 2013 at 20:39 GMT


Closing.

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

No branches or pull requests

1 participant