microsoft / WindowsAppSDK

The Windows App SDK empowers all Windows desktop apps with modern Windows UI, APIs, and platform features, including back-compat support, shipped via NuGet.
https://docs.microsoft.com/windows/apps/windows-app-sdk/
MIT License
3.8k stars 320 forks source link

RuntimeBroker.exe using about 60k per StorageItem held. #540

Open lawabider opened 3 years ago

lawabider commented 3 years ago

Sorry that this isn't really a c++/winrt issue, but it's quite desperate. The file system intermediary RuntimeBroker.exe is probably loading a thumbnail each time we create a StorageFile or StorageFolder (this is my guess as to what is happening). RuntimeBroker.exe can get to up to gigs of memory used, causing the OS to become unstable. I posted earlier here: https://docs.microsoft.com/en-us/answers/questions/246174/uwp-runtimebrokerexe-using-gigs-of-memory.html

jonwis commented 3 years ago

Hey @smaillet-ms or @aeloros - any ideas?

benstevens48 commented 3 years ago

@lawabider - yes, that's an issue with the UWP file APIs that is also a big problem for me, which I called out in my long comment here - https://github.com/microsoft/ProjectReunion/issues/8#issuecomment-631095611. I really hope that the Project Reunion team will work on fixing this soon, but I get the impression that they are putting most of their resources into supporting the release of WinUI 3 for the rest of this year.

It's possible that you can reduce the size by setting one of the indexer options here on the query options, for example OnlyUseIndexer, but I wouldn't recommend it because using the indexer is generally unreliable.

What is the reason for the large size used per file (and the slow speed of listing files)? No-one from Microsoft has ever commented on this to my knowledge. Your answer about the thumbnail is plausible. However, I have another theory, which may be completely wrong. My theory is that they decided that all file access permissions would be governed by file handles after initial creation of the StorageFile object. I'm not sure why the broker couldn't store a list of allowed paths and broker access via file path that way, but maybe it was a matter of time-saving by using the existing security mechanisms. Unfortunately, not all Win32 APIs relating to files accept a file handle, for example access to indexed properties I think. So how did they enable this scenario? Well the obvious solution is to retrieve all the data that can't be retrieved via file handle at the time the StorageFile object is created. So my conjecture is that creating a StorageFile also loads all indexed properties associated with it (and maybe that could even include a thumbnail), which explains the large file size and long time that it takes to list files. Well that's my best guess anyway, which may be completely wrong. I really hope it will be fixed sooner rather than later.

lukeblevins commented 3 years ago

@benstevens48

If you have indexer available for a location, you can set IndexerOption.OnlyUseIndexerAndOptimizeForIndexedProperties and specify only the properties you need with your storage item query and you'll get actual "partial storage items"

The only catch being that accessing any property not included in the prefetch options results in a significant performance penalty. If this fits your use-case, then read this doc page.

lawabider commented 3 years ago

@duke7553 Will the query miss files if I use that IndexerOption ? I have tried calling GetIndexedStateAsync() on a given StorageFolder, but it always returns false.

lukeblevins commented 3 years ago

@lawabider This trick doesn't work for all locations, but common locations like libraries should work.

lawabider commented 3 years ago

@benstevens48 Yes I too hope that this is fixed soon; it's really hindering me, and UWP can't be fully-fledged if we can't open a whole hard drive. Have you tried using IStorageFolderHandleAccess ? Does it make any difference?

benstevens48 commented 3 years ago

@lawabider - I believe that if you use the low level Win32 APIs such as the FindFirstFileExFromApp API along with FindNextFileW, then performance is very good (I haven't tried it myself, but there was some discussion in this issue - https://github.com/microsoft/ProjectReunion/issues/8). However, I'm not exactly sure how the permissions work with these functions. Maybe adding the storage folder to the future access list will allow FindFirstFileExFromApp to work for that folder.

@duke7553 - thanks for the tip. The indexer has not proved reliable in my experience though. Also, this is really counter-intuitively backwards - you would expect properties not to be prefetched by default and the specifying properties to prefetch to increase the time taken, not the other way around! I think this must be something to do with security making it difficult to access indexed properties after initial creation (that's the only sensible reason I can think of for this behavior), so if not prefetched then properties must be read from the file which is slow. I'm sure the APIs can be redesigned to avoid this issue given enough time (fingers crossed).

JPTGamesAndApps commented 3 years ago

@benstevens48

If you have indexer available for a location, you can set IndexerOption.OnlyUseIndexerAndOptimizeForIndexedProperties and specify only the properties you need with your storage item query and you'll get actual "partial storage items"

The only catch being that accessing any property not included in the prefetch options results in a significant performance penalty. If this fits your use-case, then read this doc page.

I had terrible experience trying to use the indexers. These are coming from my memory, but will give you some idea about the issues. If you are using it, be ready for inconsistent results. Sometimes files and missing even if the folder is supposedly fully indexed. Sometimes file extension shows up with file display name. Other times, file extension is not included in display name. Sometimes file extension does not show up in any field. Sometimes file extension shows in separate properties. Sometimes it gives extensions (.jpg) instead of description (JPEG Image). Sometimes the path is empty string....

I still have an experimental feature in my app to use indexing when available. It's been there as an experimental feature for a while and will likely be removed sometime in the future unless it starts to work better.

lukeblevins commented 3 years ago

@JPTGamesAndApps I can agree with that. The indexer-backed query options aren't as reliable as you'd expect.

On another note, let's keep this issue related to RuntimeBroker. Feel free to open another ticket for the indexer behavior and link it here so others can upvote it and weigh in.

lawabider commented 3 years ago

@benstevens48 I have just tried making a Project Reunion (PR) app and I think that std::filesystem is working (it didn't in UWP). Presumably this doesn't have the RuntimeBroker.exe problem, so it solves the problem. PR looks like a replacement for UWP, with C++/WinRT and APIs from UWP. I haven't tried the "UWP" file system APIs yet. I've only just got PR working in Visual Studio.

benstevens48 commented 3 years ago

@lawabider - thanks for the update. Yes, I'm considering migrating to a desktop app after Project Reunion becomes stable. However, I'm just really uncertain about what the future app model for Windows is. I can't help thinking that some sort of app container or sandbox has to be in their long-terms plans for reasons of security, so I don't want to invest too much in something that would be incompatible with that, but maybe they will develop a new more lightweight container as a complete replacement for the current app container and maybe I just shouldn't worry about it!

lawabider commented 3 years ago

@benstevens48 I have now written a WinUI/C++/Winrt test for both std::filesystem and WinRt APIs. For the latter there are minor differences in the code compared to UWP. https://github.com/lawabider/WinUI-C-Winrt-Test As for UWP the word is that Microsoft is "not recommending" that we create UWP projects. I think the RuntimeBroker.exe issue can be marked as closed.