search

GodotVR / godot_openxr_vendors

Godot 4 wrapper for OpenXR vendors loaders and extensions
MIT License
97 stars 22 forks source link

Use build_profile to improve build times #149

Closed dsnopek closed 3 months ago

dsnopek commented 4 months ago

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 https://github.com/godotengine/godot-cpp/pull/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 commented 4 months ago

Before this PR:

Selection_160

After this PR:

Selection_159

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

m4gr3d commented 4 months ago

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 https://github.com/godotengine/godot-cpp/pull/1167?

dsnopek commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 3 months ago

I just updated this PR for the latest changes from Fabio on PR https://github.com/godotengine/godot-cpp/pull/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 commented 3 months ago

Now that PR https://github.com/GodotVR/godot_openxr_vendors/pull/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.