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

MSIX: Performance drop on file explorer file selection #1197

Closed
arjunbalgovind opened this issue Feb 1, 2020 · 17 comments
Closed

MSIX: Performance drop on file explorer file selection #1197

arjunbalgovind opened this issue Feb 1, 2020 · 17 comments
Assignees
Labels
Area-Setup/Install Refers to installation mechanism Issue-Bug Something isn't working Product-PowerRename Refers to the PowerRename PowerToy Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@arjunbalgovind
Copy link
Contributor

After installing PowerToys using the MSIX installer, the file explorer UI lags quite a bit when selecting files. It seems to occur at time and at times it doesn't, but almost always occurs on the first use after installation. Disabling PowerRename alone doesn't help either. This performance hit doesn't occur with the MSI installer for the same set of binaries.

Steps to reproduce

  • Install using MSIX
  • Open any folder with many files and try selecting a large number with mouse, the UI will get stuck quite a bit.

Expected behavior

A gif of selecting files after installation is done using the msi installer. The UI is smooth.
powerrename_msi

Actual behavior

A gif of selecting files after installation is done using the msix installer. The UI isn't smooth.
powerrename_msix

Even if the perf hit cannot be avoided in case of MSIX, the minimum change to the code should be that disabling PowerRename (and ImageResizer once its out) and any other PowerToys which use context menu handlers should reduce the perf hit.

@arjunbalgovind arjunbalgovind added Issue-Bug Something isn't working Product-PowerRename Refers to the PowerRename PowerToy Area-Setup/Install Refers to installation mechanism labels Feb 1, 2020
@arjunbalgovind
Copy link
Contributor Author

@enricogior @crutkas Can you confirm that this isn't an issue that you are already aware of or that it isn't specific to my machine?

@crutkas
Copy link
Member

crutkas commented Feb 1, 2020

I wasn’t aware of this issue. Lemme look at it

@crutkas
Copy link
Member

crutkas commented Feb 1, 2020

What version of Windows are you on.

@enricogior
Copy link
Contributor

enricogior commented Feb 3, 2020

Hi @arjunbalgovind
what CPU are you running? I reproduced it on a 2 core laptop, but not on a 4 core desktop.
I want to make sure we have a reliable way to reproduce it in order to validate a possible fix.

@arjunbalgovind
Copy link
Contributor Author

@crutkas I'm on 18363.592.
@enricogior I'm using a 6-core desktop.

@enricogior
Copy link
Contributor

@arjunbalgovind
strange, same Windows version as yours, I tried on a 6700K that is a 4 core from 2016 and on a recent 4 core low power laptop and couldn't reproduce it. Only on a 2 core laptop from 2014 it clearly happened.

@yuyoyuppe yuyoyuppe self-assigned this Feb 4, 2020
@yuyoyuppe yuyoyuppe added the Status-In progress This issue or work-item is under development label Feb 4, 2020
@yuyoyuppe
Copy link
Contributor

yuyoyuppe commented Feb 4, 2020

I also have 6700K and the issue isn't difficult to reproduce. Perhaps the perf-drop is IO-bound, not CPU-bound. It's caused by very frequent PowerRenameUWPUI.exe relaunching - every time the file selection set changes.
Currently, WM_QUIT event is sent as soon as any method of IExplorerCommand exits, so introducing "cooling-down" timer before shutting down the process should fix the issue, since for surrogateServer we doesn't get unloaded anyway.

@enricogior
Copy link
Contributor

I should mention that on the machines that didn't show the problem I have NVMe SSDs and 32GB of ram.

@yuyoyuppe
Copy link
Contributor

I also have nvme (which overheats though) and 64gb ram. it might also be the number of running processes etc.

@yuyoyuppe
Copy link
Contributor

After investigating the issue, we need to select one of these options:

  • keep the PowerRenameUWPUI.exe in memory forever (consumes 2MB of RAM)
  • keep it in memory for X amount of minutes (possible one time lag)
  • switch to COM SurrogateServer (not suggested at this stage, it requires a significant code refactoring, it also lags and may introduces other issues/bugs like the icon not working)

@crutkas how should we proceed?

@crutkas
Copy link
Member

crutkas commented Feb 5, 2020

I think we need to think about a solution that will include image resized and others In The future such as #120. My gut says for 2mb, keep it in memory and for 0.16 we do a generic launcher so we can support more and then only have one item vs 3+ in memory.

Thoughts?

@crutkas crutkas added this to the 20.02 release milestone Feb 5, 2020
@enricogior
Copy link
Contributor

Having one extension registered for all modules is definitely something we should consider.

@arjunbalgovind
Copy link
Contributor Author

For ImageResizer, since I am implementing the IExplorerCommand interface now to support the MSIX build I'm using the SurrogateServer, since ImageResizer doesn't have a C++ exe which can be used for the exe server. It only has the C++ dll and the .NET exe. Its still in progress but it should hopefully work.

@crutkas
Copy link
Member

crutkas commented Feb 5, 2020

talking with @arunbalgovind, he mentioned a thing I missed in an earlier comment (#1197 (comment)), why is PowerRename's exe being launched each time on selection versus click? He mentioned his solution doesn't have the perf hit due to not having ImageResizer opening each time for new selection item

@yuyoyuppe
Copy link
Contributor

yuyoyuppe commented Feb 6, 2020

@crutkas

why is PowerRename's exe being launched each time on selection versus click

Explorer needs to decide whether or not to show PowerRename context menu action on selection. It must start the PowerRenameUWPUI.exe which is an exe COM server and query the command readiness with IExplorerCommand::GetState method. Finally, when the method returns, the server process exits.

Based on this comment, I'll change this behavior so the server stays in memory forever.

He mentioned his solution doesn't have the perf hit due to not having ImageResizer opening each time for new selection item

Surrogate COM server stays in memory as a dllhost.exe once loaded. I've observed that it also does have a performance hit the very first time while it's being loaded.

@crutkas
Copy link
Member

crutkas commented Feb 6, 2020

I still think we’ll need something like #1217 moving forward for unified code base and minimize system impact

@yuyoyuppe yuyoyuppe added Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed Status-In progress This issue or work-item is under development labels Feb 7, 2020
@enricogior
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Setup/Install Refers to installation mechanism Issue-Bug Something isn't working Product-PowerRename Refers to the PowerRename PowerToy Resolution-Fix Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

No branches or pull requests

4 participants