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

Test Grid Layout #79845

Closed
3 tasks done
sbatten opened this issue Aug 26, 2019 · 7 comments
Closed
3 tasks done

Test Grid Layout #79845

sbatten opened this issue Aug 26, 2019 · 7 comments

Comments

@sbatten
Copy link
Member

sbatten commented Aug 26, 2019

Testing: #50853 #79614

Complexity: 5

I've marked this issue as high complexity as I believe it should have strenuous testing to uncover any potential regressions between the new and old layout.

The grid layout is enabled by default and has been for the entire iteration. However, a perf improvement PR was merged just before endgame regarding the initial (re)construction of the workbench.

How to verify

Ensure that you haven't disabled the grid layout setting.

  1. Play with the visibility of all parts (including the editor).
  2. Play with the position of all parts (panel and sidebar).
  3. Try native and custom titlebar experiences.
  4. Please test a clean install without any cached workbench.
  5. Try setting up strange layouts, closing the window, and re-opening the workspace and verify the layout is restored as expected.
  6. Use the commands and mouse snapping behavior to hide and show parts.
  7. Work with multiple windows and workspaces.

What is cached with the layout on a workspace level and application level can be confusing when testing. Please always verify that the behavior has changed from stable when reporting a new issue. It's a good idea to use the same settings and document your steps along the way for longer repros.

@isidorn
Copy link
Contributor

isidorn commented Aug 27, 2019

I measured the startup performance of the grid with the built vscode and I could not see any measurable perfomance degradation compared to the old layouting technique. 1053.6ms for the new 1053.4 for old. So startup performance seems fine on my machine. Nice work @sbatten
fyi @jrieken

@jrieken
Copy link
Member

jrieken commented Aug 27, 2019

@isidorn Not sure what those numbers are? The perf bot still didn't show a real difference between today and yesterday: https://vscodeteam.slack.com/archives/C3NBSM7K3/p1566914782011400

@isidorn
Copy link
Contributor

isidorn commented Aug 27, 2019

@jrieken Those number are workbench ready.
Hmm between yesterday and today should be a noticable improvment, at least in theory.

@jrieken
Copy link
Member

jrieken commented Aug 27, 2019

So, you measuremented was enable/disable grid layout, right? Since the perf bot measures everything there might be something else that's now slower...

@isidorn
Copy link
Contributor

isidorn commented Aug 27, 2019

@jrieken yes. Since I can enable/disable it via the setting workbench.useExperimentalGridLayout
However I was comparing Grid layout to no grid layout.

The bot is comparing Grid from today from grid from tomorrow.
So we measure different things.

@sbatten
Copy link
Member Author

sbatten commented Aug 27, 2019

@jrieken the perf from today definitely seems better. When you originally called out the perf degradation, we were at ~1800ms and 50ms slower you said. With that, my target would have been ~1750ms which this did not hit but 1769ms is closer. But I also notice that from day to day this number has lots of variance. Looking at the last few days the spread of the 10 runs shows many slow runs whereas with today's runs the majority are at or below ~1780ms

@jrieken
Copy link
Member

jrieken commented Aug 27, 2019

Yeah, we had lost 50ms when enabling the grid but during the last days the perf bot had some fast runs (that's why I wondered when the changes have been merged). I believe @isidorn when he measured locally using the toggle-setting approach. It just seems that the perf bot lost time somewhere else... There was a pending update (which usually makes everything run slower) which is now installed

@RMacfarlane RMacfarlane removed their assignment Aug 27, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants