Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

[Performance] Improvements for QuickView #4910

Closed
redmunds opened this issue Aug 23, 2013 · 5 comments
Closed

[Performance] Improvements for QuickView #4910

redmunds opened this issue Aug 23, 2013 · 5 comments
Assignees
Milestone

Comments

@redmunds
Copy link
Contributor

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.
@peterflynn
Copy link
Member

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().

@ghost ghost assigned redmunds Aug 23, 2013
@redmunds
Copy link
Contributor Author

Nominating for Sprint 31.

@jasonsanjose
Copy link
Member

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

@redmunds
Copy link
Contributor Author

redmunds commented Sep 6, 2013

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.

@redmunds
Copy link
Contributor Author

redmunds commented Sep 6, 2013

Closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants