starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-04 05:35:20 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Stop using Windows.Storage.dll #15894

New Issue
Closed
opened 2026-01-31 04:51:32 +00:00 by claunia · 9 comments
claunia commented 2026-01-31 04:51:32 +00:00
Owner
Copy Link

Originally created by @miniksa on GitHub (Nov 15, 2021).

It's a 2.2MB ref set and it's very slow. We should try to lose it completely if we can.

Originally created by @miniksa on GitHub (Nov 15, 2021). It's a 2.2MB ref set and it's very slow. We should try to lose it completely if we can.
claunia added the Issue-TaskProduct-TerminalResolution-Won't-FixArea-Performance labels 2026-01-31 04:51:32 +00:00
claunia commented 2026-01-31 04:51:33 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Nov 15, 2021):

ACK. It's terrible, and we should be able to not use it....

That actually might be harder than expected. The IStorageItem interface is the one we use to get files off the drag/drop event, but I'm not sure there's a good legacy alternative for us to use.

@Austin-Lamb any ideas?

@zadjii-msft commented on GitHub (Nov 15, 2021): ACK. It's terrible, and we _should_ be able to not use it.... * [`TerminalPage::NewTerminalByDrop`](https://github.com/microsoft/terminal/blob/main/src/cascadia/TerminalApp/TerminalPage.cpp#L333-L361) - drag/drop file onto the new tab button * [`TerminalPage::_PasteFromClipboardHandler`](https://github.com/microsoft/terminal/blob/main/src/cascadia/TerminalApp/TerminalPage.cpp#L1809-L1818) - pasting text * [`TermControl::_DragDropHandler`](https://github.com/microsoft/terminal/blob/main/src/cascadia/TerminalControl/TermControl.cpp#L2250-L2257) - drag/dropping onto the control * `TabManagement.cpp` also `using namespace winrt::Windows::Storage::Pickers,Provider;`, but that code might be gone now. That actually might be harder than expected. The `IStorageItem` interface is the one we use to get files off the drag/drop event, but I'm not sure there's a good legacy alternative for us to use. @Austin-Lamb any ideas?
claunia commented 2026-01-31 04:51:33 +00:00
Author
Owner
Copy Link

@miniksa commented on GitHub (Nov 15, 2021):

@dmachaj had some ideas here

@miniksa commented on GitHub (Nov 15, 2021): @dmachaj had some ideas here
claunia commented 2026-01-31 04:51:33 +00:00
Author
Owner
Copy Link

@dmachaj commented on GitHub (Nov 15, 2021):

I happened to have a reference set trace of Terminal launch and I think a lot of the usage is not practical to try to avoid. The largest usage is related to destination lists (and the underlying IShellItem/IShellLink/IShellFolder) all of which are in windows.storage.dll. That is just under 1MB of reference set.

About 200KB came from SHGetKnownFolderPath and code beneath it.

400KB came from the AMD graphics driver calling something in shell32.dll that secretly redirects to windows.storage.dll. That is well outside your control and cannot be avoided completely.

@dmachaj commented on GitHub (Nov 15, 2021): I happened to have a reference set trace of Terminal launch and I think a lot of the usage is not practical to try to avoid. The largest usage is related to [destination lists](https://docs.microsoft.com/en-us/windows/win32/api/shobjidl_core/nn-shobjidl_core-icustomdestinationlist) (and the underlying IShellItem/IShellLink/IShellFolder) all of which are in windows.storage.dll. That is just under 1MB of reference set. About 200KB came from `SHGetKnownFolderPath` and code beneath it. 400KB came from the AMD graphics driver calling something in shell32.dll that secretly redirects to windows.storage.dll. That is well outside your control and cannot be avoided completely.
claunia commented 2026-01-31 04:51:34 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Nov 15, 2021):

(¬_¬ )

well that's certainly frustrating.

@zadjii-msft commented on GitHub (Nov 15, 2021): (¬_¬ ) well that's certainly frustrating.
claunia commented 2026-01-31 04:51:34 +00:00
Author
Owner
Copy Link

@dmachaj commented on GitHub (Nov 15, 2021):

🤷 It is a big OS binary that hosts a large amount of low-level shell data layer components. It is unlikely that any large project like Terminal can avoid it completely. The good news is that I didn't see any of the APIs that you are trying to avoid in the reference set 👍.

As an example here's a stack that referenced 156KB of windows.storage.dll:

 |    |- WindowsTerminal.exe!wWinMain
 |    |    WindowsTerminal.exe!AppHost::AppHost
 |    |    WindowsTerminal.exe!AppHost::_HandleCommandlineArgs
 |    |    Microsoft.Terminal.Remoting.dll!winrt::impl::produce<winrt::Microsoft::Terminal::Remoting::implementation::WindowManager,winrt::Microsoft::Terminal::Remoting::IWindowManager>::ProposeCommandline
 |    |    Microsoft.Terminal.Remoting.dll!winrt::Microsoft::Terminal::Remoting::implementation::WindowManager::ProposeCommandline
 |    |    Microsoft.Terminal.Remoting.dll!winrt::impl::invoke<winrt::Windows::Foundation::TypedEventHandler<winrt::Windows::Foundation::IInspectable,winrt::Microsoft::Terminal::Remoting::FindTargetWindowArgs>,winrt::Windows::Foundation::IInspectable,winrt::Microsoft::Terminal::Remoting::FindTargetWindowArgs>
 |    |    WindowsTerminal.exe!winrt::impl::delegate<winrt::Windows::Foundation::TypedEventHandler<winrt::Windows::Foundation::IInspectable,winrt::Windows::Foundation::IInspectable>,<lambda_b2e650f82073cf5a32476872fd0183dd> >::Invoke
 |    |    WindowsTerminal.exe!AppHost::_FindTargetWindow
 |    |    TerminalApp.dll!winrt::impl::produce<winrt::TerminalApp::implementation::AppLogic,winrt::TerminalApp::IAppLogic>::FindTargetWindow
 |    |    TerminalApp.dll!winrt::TerminalApp::implementation::AppLogic::FindTargetWindow
 |    |    TerminalApp.dll!winrt::TerminalApp::implementation::AppLogic::LoadSettings
 |    |    TerminalApp.dll!winrt::TerminalApp::implementation::AppLogic::_TryLoadSettings
 |    |    TerminalApp.dll!winrt::impl::factory_cache_entry<winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings,winrt::Microsoft::Terminal::Settings::Model::ICascadiaSettingsStatics>::call<winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings (__cdecl*)(winrt::Microsoft::Terminal::Settings::Model::ICascadiaSettingsStatics const &)>
 |    |    |- TerminalApp.dll!<lambda_bf1cf188e73bc29253cc88d4a3307376>::<lambda_invoker_cdecl>
 |    |    |    TerminalApp.dll!winrt::impl::consume_Microsoft_Terminal_Settings_Model_ICascadiaSettingsStatics<winrt::Microsoft::Terminal::Settings::Model::ICascadiaSettingsStatics>::LoadAll
 |    |    |    Microsoft.Terminal.Settings.Model.dll!winrt::impl::produce<winrt::Microsoft::Terminal::Settings::Model::factory_implementation::CascadiaSettings,winrt::Microsoft::Terminal::Settings::Model::ICascadiaSettingsStatics>::LoadAll
 |    |    |    Microsoft.Terminal.Settings.Model.dll!winrt::Microsoft::Terminal::Settings::Model::implementation::CascadiaSettings::LoadAll
 |    |    |    Microsoft.Terminal.Settings.Model.dll!winrt::Microsoft::Terminal::Settings::Model::implementation::CascadiaSettings::_SettingsPath
 |    |    |    Microsoft.Terminal.Settings.Model.dll!Microsoft::Terminal::Settings::Model::GetBaseSettingsPath
 |    |    |    Microsoft.Terminal.Settings.Model.dll!<lambda_21a2aceb9107346b7ef7ac15d5506f7c>::operator()
 |    |    |    windows.storage.dll!SHGetKnownFolderPath

That seems totally reasonable to me and not worth effort to avoid. Known folder paths are a useful concept.

@dmachaj commented on GitHub (Nov 15, 2021): 🤷 It is a big OS binary that hosts a large amount of low-level shell data layer components. It is unlikely that any large project like Terminal can avoid it completely. The good news is that I didn't see any of the APIs that you are trying to avoid in the reference set 👍. As an example here's a stack that referenced 156KB of windows.storage.dll: ``` | |- WindowsTerminal.exe!wWinMain | | WindowsTerminal.exe!AppHost::AppHost | | WindowsTerminal.exe!AppHost::_HandleCommandlineArgs | | Microsoft.Terminal.Remoting.dll!winrt::impl::produce<winrt::Microsoft::Terminal::Remoting::implementation::WindowManager,winrt::Microsoft::Terminal::Remoting::IWindowManager>::ProposeCommandline | | Microsoft.Terminal.Remoting.dll!winrt::Microsoft::Terminal::Remoting::implementation::WindowManager::ProposeCommandline | | Microsoft.Terminal.Remoting.dll!winrt::impl::invoke<winrt::Windows::Foundation::TypedEventHandler<winrt::Windows::Foundation::IInspectable,winrt::Microsoft::Terminal::Remoting::FindTargetWindowArgs>,winrt::Windows::Foundation::IInspectable,winrt::Microsoft::Terminal::Remoting::FindTargetWindowArgs> | | WindowsTerminal.exe!winrt::impl::delegate<winrt::Windows::Foundation::TypedEventHandler<winrt::Windows::Foundation::IInspectable,winrt::Windows::Foundation::IInspectable>,<lambda_b2e650f82073cf5a32476872fd0183dd> >::Invoke | | WindowsTerminal.exe!AppHost::_FindTargetWindow | | TerminalApp.dll!winrt::impl::produce<winrt::TerminalApp::implementation::AppLogic,winrt::TerminalApp::IAppLogic>::FindTargetWindow | | TerminalApp.dll!winrt::TerminalApp::implementation::AppLogic::FindTargetWindow | | TerminalApp.dll!winrt::TerminalApp::implementation::AppLogic::LoadSettings | | TerminalApp.dll!winrt::TerminalApp::implementation::AppLogic::_TryLoadSettings | | TerminalApp.dll!winrt::impl::factory_cache_entry<winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings,winrt::Microsoft::Terminal::Settings::Model::ICascadiaSettingsStatics>::call<winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings (__cdecl*)(winrt::Microsoft::Terminal::Settings::Model::ICascadiaSettingsStatics const &)> | | |- TerminalApp.dll!<lambda_bf1cf188e73bc29253cc88d4a3307376>::<lambda_invoker_cdecl> | | | TerminalApp.dll!winrt::impl::consume_Microsoft_Terminal_Settings_Model_ICascadiaSettingsStatics<winrt::Microsoft::Terminal::Settings::Model::ICascadiaSettingsStatics>::LoadAll | | | Microsoft.Terminal.Settings.Model.dll!winrt::impl::produce<winrt::Microsoft::Terminal::Settings::Model::factory_implementation::CascadiaSettings,winrt::Microsoft::Terminal::Settings::Model::ICascadiaSettingsStatics>::LoadAll | | | Microsoft.Terminal.Settings.Model.dll!winrt::Microsoft::Terminal::Settings::Model::implementation::CascadiaSettings::LoadAll | | | Microsoft.Terminal.Settings.Model.dll!winrt::Microsoft::Terminal::Settings::Model::implementation::CascadiaSettings::_SettingsPath | | | Microsoft.Terminal.Settings.Model.dll!Microsoft::Terminal::Settings::Model::GetBaseSettingsPath | | | Microsoft.Terminal.Settings.Model.dll!<lambda_21a2aceb9107346b7ef7ac15d5506f7c>::operator() | | | windows.storage.dll!SHGetKnownFolderPath ``` That seems totally reasonable to me and not worth effort to avoid. Known folder paths are a useful concept.
claunia commented 2026-01-31 04:51:34 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Nov 15, 2021):

Maybe we should loop back around on the OS side and double-tap on what value we (Windows) gets from having all these common components in one big 'ol dll? IDK I'm not sure I'm totally qualified to say I know what I'm talking about on this topic. I'm just looking for anything we can do to trim out dependencies & other startup perf.

@zadjii-msft commented on GitHub (Nov 15, 2021): Maybe we should loop back around on the OS side and double-tap on what value we (Windows) gets from having all these common components in one big 'ol dll? IDK I'm not sure I'm totally qualified to say I know what I'm talking about on this topic. I'm just looking for anything we can do to trim out dependencies & other startup perf.
claunia commented 2026-01-31 04:51:34 +00:00
Author
Owner
Copy Link

@dmachaj commented on GitHub (Nov 15, 2021):

Bigger DLLs should have better performance, all other things being equal. There are a variety of costs that are paid per-dll so fewer/bigger DLLs will pay less of those costs.

However, things can be really complicated for special cases. Windows.storage.dll is going to be loaded into almost every process so uncommonly used pieces are wasteful to page in everywhere. On the other hand the readonly pages should be easily shared across all processes so the memory hit isn't so bad for them. They'll already be paged in by someone else so there's not really any cost to someone else needing the same page; it is already available.

@dmachaj commented on GitHub (Nov 15, 2021): Bigger DLLs should have _better_ performance, all other things being equal. There are a variety of costs that are paid per-dll so fewer/bigger DLLs will pay less of those costs. However, things can be really complicated for special cases. Windows.storage.dll is going to be loaded into almost every process so uncommonly used pieces are wasteful to page in everywhere. On the other hand the readonly pages should be easily shared across all processes so the memory hit isn't so bad for them. They'll already be paged in by someone else so there's not really any cost to someone else needing the same page; it is already available.
claunia commented 2026-01-31 04:51:34 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Nov 15, 2021):

TIL! Thanks!

@zadjii-msft commented on GitHub (Nov 15, 2021): TIL! Thanks!
claunia commented 2026-01-31 04:51:35 +00:00
Author
Owner
Copy Link

@miniksa commented on GitHub (Nov 15, 2021):

Welp, thanks all for the investigation following the meeting we had earlier today. I guess we just resolve this as "MEH" for now and keep the findings for posterity.

@miniksa commented on GitHub (Nov 15, 2021): Welp, thanks all for the investigation following the meeting we had earlier today. I guess we just resolve this as "MEH" for now and keep the findings for posterity.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label Area-Performance Issue-Task Product-Terminal Resolution-Won't-Fix
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#15894
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?