Skip to content

Split bindings crate out of the main kernel crate #822

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

Merged
merged 2 commits into from
Jul 10, 2022

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Jul 9, 2022

The generated bindings are mostly static when working on new rust
abstractions. By splitting them off the main kernel crate, recompilation
of the main kernel crate takes ~2s rather than ~12s. While this loses
some optimizations without LTO, the fast majority of the binding
functions are already marked as #[inline]. Pretty much only
Default impls aren't marked as such. The cost of not inlining those
Default impls is likely neglectable.

Signed-off-by: Björn Roy Baron [email protected]

@bjorn3
Copy link
Member Author

bjorn3 commented Jul 9, 2022

I didn't add rustdoc or testing code for the bindings crate as I didn't think it would be necessary. It already wasn't documented thanks to #[doc(hidden)] and there has been no testing code anymore since my previous PR and it was not run before that anyway.

@bjorn3 bjorn3 force-pushed the split_bindings_off branch 2 times, most recently from 1022121 to 65101b9 Compare July 9, 2022 17:05
@bjorn3
Copy link
Member Author

bjorn3 commented Jul 9, 2022

How do I add a dependency on bindings to kernel for rustdoc tests?

@ojeda
Copy link
Member

ojeda commented Jul 9, 2022

How do I add a dependency on bindings to kernel for rustdoc tests?

This should solve the CI issue:

@@ -129,6 +129,9 @@ quiet_cmd_rustc_test_library = RUSTC TL $<
                -L$(objtree)/$(obj)/test \
                --crate-name $(subst rusttest-,,$(subst rusttestlib-,,$@)) $<
 
+rusttestlib-bindings: $(src)/bindings/lib.rs rusttest-prepare FORCE
+       $(call if_changed,rustc_test_library)
+
 rusttestlib-build_error: $(src)/build_error.rs rusttest-prepare FORCE
        $(call if_changed,rustc_test_library)
 
@@ -239,7 +242,7 @@ rusttest-macros: $(src)/macros/lib.rs rusttest-prepare FORCE
 rusttest-kernel: private rustc_target_flags = --extern alloc \
     --extern build_error --extern macros --extern bindings
 rusttest-kernel: $(src)/kernel/lib.rs rusttest-prepare \
-    rusttestlib-build_error rusttestlib-macros FORCE
+    rusttestlib-bindings rusttestlib-build_error rusttestlib-macros FORCE
        $(call if_changed,rustc_test)
        $(call if_changed,rustc_test_library)

@bjorn3 bjorn3 force-pushed the split_bindings_off branch from 65101b9 to 7eeb0e3 Compare July 10, 2022 08:50
bjorn3 added 2 commits July 10, 2022 11:03
The generated bindings are mostly static when working on new rust
abstractions. By splitting them off the main kernel crate, recompilation
of the main kernel crate takes ~2s rather than ~12s. While this loses
some optimizations without LTO, the fast majority of the binding
functions are already marked as `#[inline]`. Pretty much only
`Default` impls aren't marked as such. The cost of not inlining those
`Default` impls is likely neglectable.

Signed-off-by: Björn Roy Baron <[email protected]>
@bjorn3 bjorn3 force-pushed the split_bindings_off branch from 7eeb0e3 to 521024c Compare July 10, 2022 09:03
@bjorn3
Copy link
Member Author

bjorn3 commented Jul 10, 2022

CI passes now.

@ojeda ojeda merged commit 72a19d6 into Rust-for-Linux:rust Jul 10, 2022
@ojeda
Copy link
Member

ojeda commented Jul 10, 2022

Great work!

@bjorn3 bjorn3 deleted the split_bindings_off branch July 10, 2022 10:04
@kloenk
Copy link
Member

kloenk commented Jul 10, 2022

Had just a type alias on which I implemented my stuff. Guess I have to change that now, as I can't implement for types outside the crate :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants