Skip to content

Conversation

@MartyIX
Copy link
Contributor

@MartyIX MartyIX commented Mar 16, 2024

Description of Change

I have noticed that one can cache Package.Current.InstalledLocation.Path to save about 4 ms (measured in Debug mode) for each call of FileSystemUtils.PlatformGetFullAppPackageFilePath1 because there is no interop call to WinRT2.

Testing

https://github.com/MartyIX/maui/tree/feature/2024-03-16-app-path-optimization-testing (MartyIX@ab01bf4)

Issues Fixed

Contributes to #8594

Footnotes

  1. Called for each registered font (see Font loading impacts Windows desktop startup #8594).

  2. Please correct me if I got it wrong.

@MartyIX MartyIX requested a review from a team as a code owner March 16, 2024 12:51
@dotnet-policy-service
Copy link
Contributor

Hey there @MartyIX! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Mar 16, 2024
@MartyIX
Copy link
Contributor Author

MartyIX commented Mar 16, 2024

cc @jonathanpeppers

@jsuarezruiz jsuarezruiz added platform/windows legacy-area-perf Startup / Runtime performance labels Mar 18, 2024
@jonathanpeppers
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change makes sense.

In the attached issue, there was:

image

@MartyIX do you have a more recent .speedscope from your app? We should get a second data point on how much time this can save in a real application.

@jonathanpeppers
Copy link
Member

Oh, I found the .speedscope @MartyIX sent me last week:

image

Seems like an easy-win. 👍

@MartyIX
Copy link
Contributor Author

MartyIX commented Mar 18, 2024

I don't really know how to profile my real application with modified MAUI source code. So I'm testing with the sandbox project in MAUI source code.

However, I tested with MartyIX@ab01bf4 and it saved about 20 ms AFAIK. But it depends on the number of files. So if there is an app with 50 font files, then I would expect to save about 50 * <each call cost> and that can be certainly >100ms which is significant.

@jonathanpeppers
Copy link
Member

If you ever need to test a local MAUI build, I do:

  • dotnet cake --configuration=Release
  • wipe NuGet cache: rm -r ~\.nuget\packages\*\*-dev\ (if you ever do this more than once)
  • then build run an app with MAUI's .\bin\dotnet\dotnet.exe instead of the system-installed dotnet.exe

This might be trickier with a Windows app that is a "packaged" appx, though. Might have to launch VS with %PATH% set to .\bin\dotnet\.

@MartyIX
Copy link
Contributor Author

MartyIX commented Mar 26, 2024

@rmarinho Is this OK to merge? Or are there any concerns?

@rmarinho rmarinho requested review from mattleibow and removed request for StephaneDelcroix and tj-devel709 March 26, 2024 11:23
@rmarinho
Copy link
Member

pinging @mattleibow as he knows more about this that me :)

@mattleibow mattleibow merged commit 2d1bf58 into dotnet:main Mar 26, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Apr 26, 2024
@Eilon Eilon added the perf/general The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf) label May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

community ✨ Community Contribution fixed-in-8.0.20 fixed-in-9.0.0-preview.3.10457 legacy-area-perf Startup / Runtime performance perf/general The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf) platform/windows

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

7 participants