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

Cache function pointers in global tables #387

Merged
merged 9 commits into from
Sep 5, 2023

Conversation

Bromeon
Copy link
Member

@Bromeon Bromeon commented Aug 22, 2023

🔥 Detailed article here: https://godot-rust.github.io/dev/ffi-optimizations-benchmarking


Instead of repeatedly loading method pointers from Godot for each FFI call (including all the expensive StringName construction), this PR preloads all function pointers at startup. Includes:

  • Classes
  • Builtin types
  • Builtin lifecycle functions (constructors, destructors, conversion methods)
  • Global utility functions

This not only improves performance (#11), but also reveals any incompatibilitities or missing methods in Godot immediately. For example, this helped reveal godotengine/godot#80852. On the downside, a single method can prevent gdext from loading (even if that method is not used); however this likely points to an incompatibility that needs to be addressed anyway.

Other changes in this PR include:

  • StringCache for caching StringName instances during initialization
  • New init API which is easier to use and closer to what Godot provides
    • remove ExtensionLayer, InitHandle

Still missing:

  • Classes which are loaded after init-levels (currently only ThemeDB)
  • Tests for Editor init-level (our binaries don't boot the editor, so that init-level is skipped in CI)
  • Testing of ExtensionLibrary::min_level()
    • Turns out the lower layer callbacks are still called even if Scene is specified as a minimum. What is its use then?
    • Is that even correct? If not, is the new API still suitable (previous API could infer minimum based on registered layer)?

After everything's been done, the codegen crate needs a heavy refactoring. Contributors planning to change that area are recommended to wait a bit.

@Bromeon Bromeon added feature Adds functionality to the library c: engine Godot classes (nodes, resources, ...) labels Aug 22, 2023
@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-387

@Bromeon Bromeon added this pull request to the merge queue Aug 26, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 26, 2023
# Conflicts:
#	godot-macros/src/method_registration/mod.rs
#	godot-macros/src/method_registration/register_method.rs
#	godot-macros/src/method_registration/virtual_method_callback.rs
@Bromeon Bromeon added this pull request to the merge queue Sep 5, 2023
Merged via the queue into master with commit 9e6fe56 Sep 5, 2023
14 checks passed
@Bromeon Bromeon deleted the qol/cached-function-pointers branch September 5, 2023 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: engine Godot classes (nodes, resources, ...) feature Adds functionality to the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants