Skip to content

Conversation

@Bromeon
Copy link
Member

@Bromeon Bromeon commented Oct 19, 2023

Instead of storing all function pointers in a huge array in static/global storage, a Vec is now used, employing the heap. Should help with compile times, memory usage (LLVM release mode) and WASM experiments.

Furthermore, this adds the Cargo feature lazy-function-tables, which switches the mechanism: a HashMap lazily loads function pointers upon first use. This was done in part to help with WebAssembly experiments and possibly supersedes #459.

Lazy loading however comes with several downsides, so we discourage its use unless there are very good reasons:

  1. Missing functions are only detected at runtime rather than startup time, possibly in conditional branches that aren't hit during development. This has potential for nasty bugs once software is already shipped.
  2. Threading is currently unsupported. And once it will be supported, it will incur lock contention (one single lock for all possible functions).
    • If we support it, we may need to fetch pointers from the main thread (unclear whether this GDExtension API is multi-threaded).
    • Thread-local tables are a potential alternative, but fetching itself may still need locking (see previous point).
  3. There is likely a performance impact for each FFI call -- to be measured.
  4. Startup times are already extremely fast, so there's no noteworthy benefit here.

We may also downgrade this from a public feature to an implementation detail that's only selected for certain configurations.

@Bromeon Bromeon added quality-of-life No new functionality, but improves ergonomics/internals c: ffi Low-level components and interaction with GDExtension API labels Oct 19, 2023
@GodotRust
Copy link

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

@Bromeon Bromeon force-pushed the qol/no-stack-fptr-tables branch from 7edd188 to 312c0aa Compare October 22, 2023 20:54
@Bromeon Bromeon added feature Adds functionality to the library and removed quality-of-life No new functionality, but improves ergonomics/internals labels Oct 22, 2023
@Bromeon Bromeon changed the title Use Vec instead of array for global function tables Global function tables: Vec instead of array, optional lazy loading Oct 22, 2023
@Bromeon Bromeon force-pushed the qol/no-stack-fptr-tables branch 2 times, most recently from 26baf96 to ded7472 Compare October 22, 2023 21:10
@Bromeon Bromeon force-pushed the qol/no-stack-fptr-tables branch from ded7472 to ea7b70e Compare October 22, 2023 21:18
@Bromeon Bromeon added this pull request to the merge queue Oct 22, 2023
Merged via the queue into master with commit b99fe33 Oct 22, 2023
@Bromeon Bromeon deleted the qol/no-stack-fptr-tables branch October 22, 2023 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: ffi Low-level components and interaction with GDExtension API feature Adds functionality to the library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants