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

Use build_profile to improve build times #149

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

dsnopek
Copy link
Collaborator

@dsnopek dsnopek commented May 15, 2024

Since I've been working on CI this week, the fact that this repo builds painfully slow on GitHub Actions was starting to get to me :-)

This PR uses godot-cpp PR godotengine/godot-cpp#1167 to only build the classes that are actually used. Unfortunately, because we make an editor plugin and load GLTF's, we do actually use quite a few classes. However, this still seems to cut build times by 2-4x!

Marking as a draft for now, because I'm not sure if this is the right way to integrate it. Currently, I'm including a patch for godot-cpp, and while the build_profile.json is built via a script, it's committed to the repo.

@dsnopek dsnopek added the enhancement New feature or request label May 15, 2024
@dsnopek dsnopek added this to the 3.0.0 milestone May 15, 2024
@dsnopek dsnopek requested a review from m4gr3d May 15, 2024 20:52
@dsnopek
Copy link
Collaborator Author

dsnopek commented May 15, 2024

Before this PR:

Selection_160

After this PR:

Selection_159

So, it's like a 2-4x improvement

@dsnopek dsnopek marked this pull request as draft May 15, 2024 21:35
@m4gr3d
Copy link
Collaborator

m4gr3d commented May 15, 2024

while the build_profile.json is built via a script

What's the process / script command for building build_profile.json?

Currently, I'm including a patch for godot-cpp

Is there a blocker for merging godotengine/godot-cpp#1167?

@dsnopek
Copy link
Collaborator Author

dsnopek commented May 15, 2024

What's the process / script command for building build_profile.json?

The create_build_profile.py script in this PR is what I used to build it. It would probably be good to have something like that in godot-cpp itself, but I'm not sure the best way to package it up for developers.

Is there a blocker for merging godotengine/godot-cpp#1167?

There's certainly more that could be done with it, but I personally think it's helpful enough as-is to merge and improve later. Fabio has it marked as a draft still, though - I pinged him on a comment there asking what he thinks still needs to be done.

@m4gr3d
Copy link
Collaborator

m4gr3d commented May 15, 2024

What's the process / script command for building build_profile.json?

The create_build_profile.py script in this PR is what I used to build it. It would probably be good to have something like that in godot-cpp itself, but I'm not sure the best way to package it up for developers.

The script could be automatically invoked via a scons argument.

For example, the build_profile argument allows to manually specify a custom build profile configuration, another argument can be added to automatically invoke the script to generate a build profile configuration, and then use that configuration automatically.

@dsnopek
Copy link
Collaborator Author

dsnopek commented May 15, 2024

The script could be automatically invoked via a scons argument.

For example, the build_profile argument allows to manually specify a custom build profile configuration, another argument can be added to automatically invoke the script to generate a build profile configuration, and then use that configuration automatically.

Because it's parsing the .hpp and .cpp files that godot-cpp generates, it won't work until after it's had a chance to generate them. And, by using a build profile, it will prevent some .cpp files from being generated. :-) So, there's a sort of circular dependency on having run scons at least once already without a build profile, before we can generate the build profile. That's OK for a project-specific script, but I'd want it to be more automatic if it were included in godot-cpp itself.

It'd be theoretically possible to figure out the dependencies between the engine classes from the extension_api.json alone, but it requires a whole bunch of processing which binding_generator.py is already doing to produce the .hpp and .cpp files, which is why I built the script in this PR the way I did.

@dsnopek
Copy link
Collaborator Author

dsnopek commented Jun 15, 2024

I just updated this PR for the latest changes from Fabio on PR godotengine/godot-cpp#1167

Because it's parsing the .hpp and .cpp files that godot-cpp generates, it won't work until after it's had a chance to generate them. And, by using a build profile, it will prevent some .cpp files from being generated. :-)

In the recent changes, Fabio made it a little smarter, which allowed me to remove the .cpp parsing from create_build_profile.py (now it only parses the .hpp files) which makes this situation a little better. However, it is still a two-pass process because of the way godot-cpp's scons configuration works. I think we could continue to make this better over time, though.

@dsnopek
Copy link
Collaborator Author

dsnopek commented Jun 20, 2024

Now that PR #171 has been merged and the build_profile support is in godot-cpp, I'm going to take this out of draft for further review. It's not perfect, but it is a very worthwhile improvement in build times.

@dsnopek dsnopek changed the title [DRAFT] Use build_profile to improve build times Use build_profile to improve build times Jun 20, 2024
@dsnopek dsnopek marked this pull request as ready for review June 20, 2024 17:45
@m4gr3d m4gr3d merged commit 10cf401 into GodotVR:master Jun 24, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants