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

[Bindings] Build profile now strips methods and skip files #1680

Merged
merged 2 commits into from
Jan 7, 2025

Conversation

Faless
Copy link
Contributor

@Faless Faless commented Jan 2, 2025

This allows removing dependencies that are not explicitly unused by the gdextension being built and is implemented using an intermediate json API file with the methods and classes stripped (i.e. without touching the file generators).

Alternative to #1675

@Faless Faless added enhancement This is an enhancement on the current functionality needs testing topic:buildsystem Related to the buildsystem or CI setup labels Jan 2, 2025
@Faless Faless requested review from a team as code owners January 2, 2025 14:51
@Faless Faless force-pushed the build/profile_strip_json branch 2 times, most recently from fec7bfe to ebf9829 Compare January 3, 2025 14:28
@Faless
Copy link
Contributor Author

Faless commented Jan 3, 2025

I've further extracted the functions related to build profiles into a dedicated tool (build_profile.py) that can also be invoked as standalone to generate the trimmed API (either saving it to file or printing it to stdout)

@Faless Faless force-pushed the build/profile_strip_json branch 2 times, most recently from 81ee264 to 37487dc Compare January 3, 2025 16:20
@dsnopek
Copy link
Collaborator

dsnopek commented Jan 6, 2025

@Faless Between this PR and #1675, which is your preference?

The experience for users of scons is the same, and it sounds (from the discussion on the other PR) like either approach is fine for CMake as well.

I'm personally fine with either, although, I guess this PR has some theoretical advantages, like other language bindings could use it, or if maybe there start being other tools that interact with extension_api.json files.

@Faless
Copy link
Contributor Author

Faless commented Jan 6, 2025

Between this PR and #1675, which is your preference?

I wasn't really convinced by this approach at first, but after a couple of iteration I think it's now better than the other, and indeed it should be usable as a standalone tool by other bindings. I'll close the other PR.

Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

This looks really great to me!

In the past, we've discussed that adding the ability to exclude methods would be impractical because you'd need to list all the methods, but this approach is a very nice compromise.

I tested with the test project here, and the godot_openxr_vendors extension, and the debug binaries were about ~200k and ~400k smaller, respectively.

Other than the one issue I noted with 'godot_openxr_vendors' below, it seemed to work great! I skimmed the code, and that's looking good too

build_profile.py Outdated Show resolved Hide resolved
@Faless Faless force-pushed the build/profile_strip_json branch from 37487dc to 5ea86b4 Compare January 7, 2025 12:44
Copy link
Collaborator

@dsnopek dsnopek left a comment

Choose a reason for hiding this comment

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

Thanks! LGMT :-)

build_profile.py Outdated Show resolved Hide resolved
Faless added 2 commits January 7, 2025 20:33
This allows removing dependencies that are not explicitly unused by the
gdextension being built and is implemented using an intermediate json
API file with the methods and classes stripped (i.e. without touching
the file generators).
@Faless Faless force-pushed the build/profile_strip_json branch from 825c2dd to 0cfe01e Compare January 7, 2025 19:33
@dsnopek dsnopek merged commit 7d3870b into godotengine:master Jan 7, 2025
11 checks passed
@Faless Faless deleted the build/profile_strip_json branch January 8, 2025 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick:4.2 cherrypick:4.3 enhancement This is an enhancement on the current functionality topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants