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

Add #[inline] to almost all of the public API for every crate #398

Open
blaumeise20 opened this issue Mar 2, 2024 · 3 comments
Open

Add #[inline] to almost all of the public API for every crate #398

blaumeise20 opened this issue Mar 2, 2024 · 3 comments

Comments

@blaumeise20
Copy link
Contributor

It is a well known "problem" in Rust that automatic cross-crate inlining is not supported. Every function that is seen as good to inline by the programmer has to be annotated with #[inline] to basically allow the Rust compiler to do inlining across crate boundaries. If you look into the standard library almost every function is marked as inline, however I haven't really seen that here. Inlining can have a great impact on performance, so we should consider to add #[inline] to pretty much the entire public API of the crates.

This page has a good explanation on how to configure inlining: https://nnethercote.github.io/perf-book/inlining.html

@Sharktheone
Copy link
Member

Inlining should be done automatically by the compiler even if it hasn't the "permission" because we have enabled fat lto and have set codegen units to one in #310. It's enabled in the root crate, so it should also affect other crates that live in crates/.

But I'm not sure of is how that affects potential embedders of the engine. So it might be still needed to mark public functions as #[inline(always), but this is something I need to test.

@blaumeise20
Copy link
Contributor Author

Yeah that's what I assumed. With "root crate" you mean the binaries? If not, then anywhere the crates are included the configuration of those root crates will count.
I'm not sure if it is a good idea to add always to the inline directive, this is really only for small internal helper functions. #[inline] itself is mostly enough because the Rust compiler is really smart in this case. Just look at the standard library, I don't think there is any always at all.

@Sharktheone
Copy link
Member

The root crate is gosub_engine which also contains the binaries, yes. But probably it's still good to have this configuration in every crate in case someone only uses one of these crates.

The stdlib also uses #[inline(always] for example in the Rc file there are 81 inline calls and 11 of those are #[inline(always] Another example is v8 they seem to only use [inline(always)].

I generally also think it shouldn't really be a problem when (almost) every function is inlined. Fat lto should from what I have read also do that.

@jaytaph What do you think?

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

No branches or pull requests

2 participants