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

Visual Studio: Improve performance of parsing project file #88949

Merged
merged 1 commit into from
Feb 29, 2024

Conversation

shana
Copy link
Contributor

@shana shana commented Feb 28, 2024

Visual Studio doesn't handle string parsing very well, so having all the files in one property slows down VS a lot when loading the projects. Splitting the files up into per-directory properties brings down project processing times from 20 seconds to 1 second (on my machine).

Also adding a vsproj_props_only flag to skip touching the vcxproj and sln files when regenerating the project files. This is so the rebuild command in VS can regenerate the build-specific project files without throwing away user customizations to the project. This flag is off by default, and only enabled in the Rebuild All command from VS (or if the user passes it on the command line).

Note: All the build-specific information is in the generated.props files, and the vcxproj and sln files only contain the generic list of platform configurations available to VS. The only time both sln and vcxproj file can change is when adding new platforms, and the only time the vcxproj file can change (but not the sln) is when adding/removing files (and this doesn't affect the build itself, but only the file listing in the VS Solution Explorer). All other build system changes only affect the .generated.props files. Therefore, most of the time, it's safe to regenerate only the generated.props files and not touch the vcxproj or sln files.

@akien-mga akien-mga changed the title VS: Improve performance VS: Improve performance of parsing project file Feb 28, 2024
@akien-mga
Copy link
Member

akien-mga commented Feb 28, 2024

A more explicit commit message would be good (e.g. like I edited the PR too, if that's accurate). Otherwise there are lots of things I can think of as "VS performance" which wouldn't really relate to that (e.g. the performance of binaries compiled with MSVC, or the speed of compilation with VS/MSVC).

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Some style issues to fix by running black -l 120 on the file (or installing the precommit hooks with pip install pre-commit && pre-commit install), but otherwise looks good!

@shana
Copy link
Contributor Author

shana commented Feb 28, 2024

Oh hey, thanks @akien-mga! I'm just running some final tests to make sure nothing is broken (looks pretty good so far), and I'll clean it up 👍

@shana shana force-pushed the vs-performance-improvement branch from 1df24b3 to db394af Compare February 28, 2024 11:00
VS doesn't handle string parsing very well, so having all the files in one
property slows down VS a lot when loading the projects. Splitting the files
up into per-directory properties brings down project processing times from
20 seconds to 1 second (on my machine).
@shana shana force-pushed the vs-performance-improvement branch from db394af to d6f2bec Compare February 28, 2024 12:23
@shana shana marked this pull request as ready for review February 28, 2024 12:35
@shana shana requested a review from a team as a code owner February 28, 2024 12:36
@shana
Copy link
Contributor Author

shana commented Feb 28, 2024

Ok, tested it all, good to go! 👍

@aaronfranke aaronfranke changed the title VS: Improve performance of parsing project file Visual Studio: Improve performance of parsing project file Feb 29, 2024
@akien-mga akien-mga merged commit d9296e5 into godotengine:master Feb 29, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants