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

Discover NIFs at startup #613

Merged
merged 7 commits into from
Jun 7, 2024
Merged

Discover NIFs at startup #613

merged 7 commits into from
Jun 7, 2024

Conversation

filmor
Copy link
Member

@filmor filmor commented May 28, 2024

This is a proof-of-concept for a mechanism that does away with the need to list all NIF functions in the rustler::init! invocation. A similar mechanism will be required and implemented for resource type registrations.

  • Convert Nif to a struct
  • Use inventory to register the Nif instances
  • In rustler::init!, ignore the passed functions and instead use the registered Nif instances

Next steps:

  • See whether linkme is not a better option (creates the array directly at build-time, no need to leak) (/edit Looking at this again, don't think this adds much, the current implementation is fine)
  • Implement resource registration using this mechanism (Refactor Resources #617)
  • See if we can move the macros to rustler (might do this in another iteration, no need to put it in the same PR)

This is a proof-of-concept for a mechanism that does away with the need
to list all NIF functions in the `rustler::init!` invocation. A similar
mechanism will be required and implemented for resource type
registrations.

- Convert `Nif` to a `struct`
- Use `inventory` to register the `Nif` instances
- In `rustler::init!`, ignore the passed functions and instead use the
  registered `Nif` instances

Next steps:

- See whether `linkme` is not a better option (creates the array
  directly at build-time, no need to leak)
- Implement resource registration using this mechanism
- See if we can move the macros to `rustler`
@scrogson
Copy link
Member

Love this idea!

@filmor filmor marked this pull request as ready for review June 3, 2024 10:14
@filmor filmor requested review from evnu and scrogson June 3, 2024 10:17
The behaviour to be tested had been moved to the other NIF in the same
module without removing the unused function.
@filmor
Copy link
Member Author

filmor commented Jun 6, 2024

From my point of view, this is done and ready to merge.

Copy link
Member

@scrogson scrogson left a comment

Choose a reason for hiding this comment

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

:shipit:

@filmor filmor requested a review from hansihe June 6, 2024 21:24
@filmor filmor mentioned this pull request Jun 7, 2024
@filmor
Copy link
Member Author

filmor commented Jun 7, 2024

I'll merge this now. We can further discuss on #620 and/or #617 whether and how to use linkme instead.

@filmor filmor merged commit 868c01b into rusterlium:master Jun 7, 2024
34 checks passed
@filmor filmor deleted the discovery branch June 7, 2024 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants