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.

Sign up for GitHub

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

Jump to bottom

Sprinkle #[inline] across the library #1318

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

Sprinkle #[inline] across the library #1318

wants to merge 23 commits into from

Conversation

wprzytula
Copy link
Collaborator

@wprzytula wprzytula commented Apr 12, 2025 •
edited
Loading

What's done

I went through all the code in our library and added #[inline] attribute to pub functions that are short enough to make sense to be inlined.

Note that here are no changes to scylla-macros, because they only emit functions that are arbitrarily large (depending on the number of fields of a custom struct passed to the macros).

Results

These are the changes in the binary size (left = after, right = before):

❯ diff -y <(ls target/release/deps/integration-67e7ceb0bf5d0a39 --size) <(ls target/release-non-inline/deps/integration-67e7ceb0bf5d0a39 --size)
25072 target/release/deps/integration-67e7ceb0bf5d0a39	      |	25032 target/release-non-inline/deps/integration-67e7ceb0bf5d

❯ diff -y <(ls target/release/ --size) <(ls target/release-non-inline/ --size)
total 22016						      |	total 21916
   0 build							   0 build
   0 deps							   0 deps
   0 examples							   0 examples
   0 incremental						   0 incremental
   4 libscylla_cql.d						   4 libscylla_cql.d
7820 libscylla_cql.rlib					      |	7752 libscylla_cql.rlib
   8 libscylla.d						   8 libscylla.d
   4 libscylla_macros.d						   4 libscylla_macros.d
4120 libscylla_macros.so					4120 libscylla_macros.so
   4 libscylla_proxy.d						   4 libscylla_proxy.d
1256 libscylla_proxy.rlib					1256 libscylla_proxy.rlib
8800 libscylla.rlib					      |	8768 libscylla.rlib

Conclusion

As you can see, there is a slight increase in the binary size of both libraries, as well as the integration test suite.
This is something that I'd expect if my changes do affect the compiler's inlining decisions.

Fixes: #949

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • [ ] I added relevant tests for new features and bug fixes.
  • All commits compile, pass static checks and pass test.
  • PR description sums up the changes and reasons why they should be introduced.
  • [ ] I have provided docstrings for the public items that I want to introduce.
  • [ ] I have adjusted the documentation in ./docs/source/.
  • I added appropriate Fixes: annotations to PR description.

wprzytula added 23 commits April 12, 2025 13:29
@wprzytula
deserialize: sprinkle #[inline] onto pub fns
b5db106 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
serialize: sprinkle #[inline] onto pub fns
4fc0c26 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
frame/request: sprinkle #[inline] onto pub fns
13b05c6 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
frame/response: sprinkle #[inline] onto pub fns
0f2827a 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
frame: sprinkle #[inline] onto pub fns
c0d7601 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
value: sprinkle #[inline] onto pub fns
b840930 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
session: merge Session's impl blocks
760d5f4 
As a side effect of the deserialization refactor, there emerged two
separate `impl` blocks for `Session`. This commit merges them, because
they aren't separated in any meaningful way.
@wprzytula
session: sprinkle #[inline] onto short API fns
da27942 
This allows inlining across crates and so may bring performance gains
to the users. After all, the decision whether to inline or not is left
for the compiler, so it should never hurt.
@wprzytula
session_builder: sprinkle #[inline] onto pub fns
ce41efc 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
node: sprinkle #[inline] onto short API fns
fbc91fb 
This allows inlining across crates and so may bring performance gains
to the users. After all, the decision whether to inline or not is left
for the compiler, so it should never hurt.
@wprzytula
cluster/state: sprinkle #[inline] onto short API fns
531e7c8 
This allows inlining across crates and so may bring performance gains
to the users. After all, the decision whether to inline or not is left
for the compiler, so it should never hurt.
@wprzytula
statements: sprinkle #[inline] onto pub fns
4d4e4bf 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
prepared_statement: add missing docstrings
2c9c98f
@wprzytula
locator: sprinkle #[inline] onto pub fns
b36cbd7 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
self_identity: sprinkle #[inline] onto pub fns
9393e05 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
caching_session: sprinkle #[inline] onto pub fns
f4e5d4c 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
execution_profile: sprinkle #[inline] onto pub fns
43b2db8 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
metrics: sprinkle #[inline] onto pub fns
7dc2124 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
retry_policy: sprinkle #[inline] onto pub fns
fdabd17 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
host_filter: sprinkle #[inline] onto pub fns
f85b65a 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
speculative_execution: sprinkle #[inline] onto pub fns
7c71501 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
timestamp_generator: sprinkle #[inline] onto pub fns
71380c1 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula
sharding: sprinkle #[inline] onto pub fns
5ff333a 
This allows inlining across crates and so may bring performance gains to
the users. After all, the decision whether to inline or not is left for
the compiler, so it should never hurt.
@wprzytula wprzytula self-assigned this Apr 12, 2025
@wprzytula wprzytula added enhancement New feature or request performance Improves performance of existing features and removed enhancement New feature or request labels Apr 12, 2025
@wprzytula wprzytula added this to the 1.2.0 milestone Apr 12, 2025
@github-actions GitHub Actions
Copy link

cargo semver-checks found no API-breaking changes in this PR.
Checked commit: 5ff333a

@wprzytula wprzytula requested a review from Lorak-mmk April 12, 2025 11:54
@wprzytula wprzytula requested a review from muzarski April 12, 2025 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lorak-mmk Lorak-mmk Awaiting requested review from Lorak-mmk

@muzarski muzarski Awaiting requested review from muzarski

At least 1 approving review is required to merge this pull request.

Assignees

@wprzytula wprzytula

Labels
performance Improves performance of existing features
Projects
None yet
Milestone
1.2.0
Development

Successfully merging this pull request may close these issues.

Sprinkle #[inline] across scylla-cql crate to allow inlining into scylla crate
1 participant
@wprzytula