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] Unified "launcher" for shell context apps due to perf issue #1217

Closed
crutkas opened this issue Feb 5, 2020 · 3 comments
Closed

[MSIX] Unified "launcher" for shell context apps due to perf issue #1217

crutkas opened this issue Feb 5, 2020 · 3 comments
Labels
Area-Quality Stability, Performance, Etc. Product-Image Resizer Things regarding image resizing module Product-PowerRename Refers to the PowerRename PowerToy
Milestone

Comments

@crutkas
Copy link
Member

crutkas commented Feb 5, 2020

From issue #1197, we saw that with MSIX and a lot of the scenarios PowerToys would interact with, we couldn't do the typical solution we would with MSI.

For the short term, we loaded the single item into memory however for a better, longer term solution, we must have a solution that works with multiple scenarios and each scenario isn't loaded into memory as well.

My thought is we have a thin, lightweight launcher that is in memory that then delegates out the task to the actual executable

@crutkas crutkas added Product-PowerRename Refers to the PowerRename PowerToy Area-Quality Stability, Performance, Etc. Product-Image Resizer Things regarding image resizing module labels Feb 5, 2020
@crutkas crutkas added this to the 20.03 release milestone Feb 5, 2020
@yuyoyuppe yuyoyuppe self-assigned this Feb 6, 2020
@enricogior enricogior removed this from the 20.03 release milestone Feb 27, 2020
@yuyoyuppe
Copy link
Contributor

My thought is we have a thin, lightweight launcher that is in memory that then delegates out the task to the actual executable

I agree with you on this architecture, though I wonder how this differs from #1051.

I'd like to clarify both of these issues.

Ways to register shell extensions

The way shell extensions work for MSIX is by associating some COM server (dll/exe) and class GUID implementing IExplorerCommand interface by declaring them in the Appx manifest. If we declare multiple desktop4:FileExplorerContextMenus/desktop4:VerbS which point to the same class, we won't be able to distinguish between those commands. I'll need to verify that for 100% confidence, but it pretty much looks like there's no non-hacky way to do it. Since we want to extract the common parts of IExplorerCommand interface implementation, but also have a "thin, lightweight launcher", we could extract some helpers for GetIcon and launching an executable I guess, but not much else.

I'd like to talk about some technical considerations here first, so we can clarify what we could do with memory consumption/being issues.

MSI/MSIX shell registration differences

  • For MSI, PowerRename doesn't have any executables, because PowerRenameExt.dll is directly loaded in explorer.exe process space. So, PowerRename launched a GUI thread in explorer.exe, which joined when a PowerRename GUI window
  • For MSIX, we must use dllhost.exe proxy or a separate exe server to provide IExplorerCommand implementation class

Surrogate COM server and EXE server differences

  • The main difference between surrogate COM server (which uses dllhost.exe) and EXE server (which uses an executable we provide) is that we can't directly control when the dllhost.exe shuts us down. There're SetProcessReference/GetProcessReference functions, but they have issues as well.
  • When IExplorerCommand::Invoke is called, we must serialize psiItemArray contents somewhere so the executable we launch can later deserialize and use them. We don't have to do that for the EXE server, since we can launch a thread and use those values directly. Also, PowerRename extensively used those values internally, and that's main reason I've used EXE server for it as part of MSIX adoption.

Summary

  • we can easily control the exact moment we unload the EXE server, for example, depending on whether a user has disabled a module in the PT settings
  • we don't have to serialize shell items info for EXE server, but if a module consumes a significant amount of memory, we also have an option to preprocess them before serialization

So, I suggest we implement a base class which implements a basic EXE COM server and a base class implementing IExplorerCommand interface where possible.

Thoughts?

@crutkas
Copy link
Member Author

crutkas commented Mar 4, 2020

My thoughts here is, this is an MSIX issue, lets hold on this and focus on other tasks. I think we should focus on MSI but keep in back of our minds how we enable MSIX for now.

I still like the idea of a single unified way to register since there will be more shell extensions in the future (#1051)

@crutkas crutkas added this to the Backlog milestone Mar 11, 2020
@crutkas crutkas changed the title Unified "launcher" for shell context apps due to perf issue [MSIX] Unified "launcher" for shell context apps due to perf issue Mar 13, 2020
@crutkas
Copy link
Member Author

crutkas commented May 14, 2020

we'll reopen once MSIX is back up as a tracking item

@crutkas crutkas closed this as completed May 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Quality Stability, Performance, Etc. Product-Image Resizer Things regarding image resizing module Product-PowerRename Refers to the PowerRename PowerToy
Projects
None yet
Development

No branches or pull requests

3 participants