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 tags to commonly used functions #475

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

joshuataylor
Copy link

I've gone through and added inline to some commonly used functions.

If everyone is happy, we could also add these to others.

We should judge what should be inlined. See https://nnethercote.github.io/perf-book/inlining.html

None. The compiler will decide itself if the function should be inlined. This will depend on the optimization level, the size of the function, etc. If you are not using link-time optimization, functions will never be inlined across crates.
#[inline]. This suggests that the function should be inlined, including across crate boundaries.

Amazingly I saw great wins with just this, as I have to call some of these functions a lot when encoding a lot of data.

This is using lto:
before

Operating System: macOS
CPU Information: Apple M1
Number of Available Cores: 8
Available memory: 16 GB
Elixir 1.13.4
Erlang 25.0.2

Benchmarking encode column ...

Name                    ips        average  deviation         median         99th %
encode columns         13.86       72.17 ms    ±21.40%       74.47 ms      114.95 ms

after:

Name                    ips        average  deviation         median         99th %
encode columns         15.12       66.13 ms    ±23.48%       68.36 ms      108.11 ms

@evnu evnu requested a review from a team July 27, 2022 11:56
@evnu
Copy link
Member

evnu commented Aug 1, 2022

@joshuataylor thanks for the PR. Can you share the code for your benchmark?

@@ -118,6 +118,7 @@ pub fn nif(args: TokenStream, input: TokenStream) -> TokenStream {
/// such that you can use the Elixir struct definition for it.

#[proc_macro_derive(NifStruct, attributes(module, rustler))]
#[inline]
Copy link
Member

Choose a reason for hiding this comment

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

This is run at compile time, I don't see a point in inlining it.

@hansihe
Copy link
Member

hansihe commented Sep 11, 2022

This looks like a very good idea to me.

@evnu
Copy link
Member

evnu commented Nov 11, 2022

Interesting. I would not expect inline to make a real difference. Can we explain why we see a difference in the performance?

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.

4 participants