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.

Sign up for GitHub

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

Jump to bottom

Find widget's checkEditorWidth does synchronous layout on editor resize #34744

Closed
alexdima opened this issue Sep 21, 2017 · 2 comments
Closed

Find widget's checkEditorWidth does synchronous layout on editor resize #34744

alexdima opened this issue Sep 21, 2017 · 2 comments
Assignees
@rebornix
Labels
bug Issue identified by VS Code Team member as probable bug editor-find Editor find operations perf verified Verification succeeded
Milestone
November 2017

Comments

@alexdima
Copy link
Member

alexdima commented Sep 21, 2017 •
edited
Loading

fyi @rebornix

Discovered while looking into #34716

  • have a diff editor (side by side)
  • resize via sash:
    kapture 2017-09-21 at 12 08 09

Observe a lot of time being spent in checkEditorWidth:

image
@alexdima alexdima self-assigned this Sep 21, 2017
@vscodebot vscodebot bot added editor editor-find Editor find operations labels Sep 21, 2017
@alexdima alexdima added perf editor-find Editor find operations and removed editor editor-find Editor find operations labels Sep 21, 2017
@alexdima alexdima assigned rebornix and unassigned rebornix and alexdima Sep 21, 2017
This was referenced Sep 21, 2017
Closed
Closed
@rebornix rebornix added the bug Issue identified by VS Code Team member as probable bug label Nov 16, 2017
@rebornix rebornix mentioned this issue Nov 16, 2017
Closed
mjbvz added a commit to mjbvz/vscode that referenced this issue Nov 17, 2017
@mjbvz
Don't try resizing find widget when it is not visible
a20cdb4 
Part of microsoft#34744

Don't try to update the width of the find widget when it is not visible. This update function is slow as it relies on `dom.getTotalWidth`
@mjbvz mjbvz mentioned this issue Nov 17, 2017
Merged
@alexdima
Copy link
Member Author

@rebornix There's a PR from @mjbvz that improves this -- #38651

@rebornix
Copy link
Member

Mitigate issue by not checking width/dom-info when users never resize the find widget (the usual case, most ppl don't resize at all). Then the DOM related operations are gone

image

It looks good now for this particular problem. We can continue improving this by listening to ResizeObserver as Atom does but it requires experimentalWebFeatures in Electron rendering process and it only helps inside VSCode.

@rebornix rebornix added this to the November 2017 milestone Nov 27, 2017
@bpasero bpasero added the verified Verification succeeded label Dec 6, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@rebornix rebornix

Labels
bug Issue identified by VS Code Team member as probable bug editor-find Editor find operations perf verified Verification succeeded
Projects
None yet
Milestone
November 2017
Development

No branches or pull requests
3 participants
@rebornix @bpasero @alexdima