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

Consider exposing different flavors of channels using different types. #979

Open
EFanZh opened this issue Apr 28, 2023 · 9 comments
Open

Comments

@EFanZh
Copy link

EFanZh commented Apr 28, 2023

In crossbeam, there are many flavors of channels: array, list, zero, etc., but they are implementation details, and all hidden under the Sender and Receiver type. The problem is that many times, I need to use a certain type of channel, say an unbounded channel for a Foo type, the compiler has to generate codes for all kinds of flavors, since the Receiver type needs to support all types of channels: https://docs.rs/crossbeam-channel/0.5.8/src/crossbeam_channel/channel.rs.html#812-838. This could lead to bloated binary size.

Is it possible to provide different types for different flavors of channels? So I only need to pay for what I use.

If API compatibility is an issue, how about adding a new module for these different flavors of channels? Or even add a new crate, then base crossbeam-channel on this new crate?

@taiki-e
Copy link
Member

taiki-e commented Apr 28, 2023

Have you actually measured this in a situation where fat LTO is enabled?

The compiler may recognize that other branches are not being used and can remove unused branches. (rust-lang/rust#107198 (comment))

If it works, we should be able to get the same optimization without LTO by adding a few inline attributes. And your request should be solved without duplicating a lot of APIs, confusing beginners with a lot of similar APIs, or putting the maintainer through the task of making sure the APIs are consistent across flavors.

@taiki-e
Copy link
Member

taiki-e commented Apr 28, 2023

adding a few inline attributes

Well, since the channel has generics, the inline attribute may not actually be needed.

@taiki-e taiki-e added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author label Apr 28, 2023
@EFanZh
Copy link
Author

EFanZh commented Apr 28, 2023

For one of my project, after testing a few configurations, lto=thin and opt-level=z are chosen as the best configuration for me. And under these optimization flags, replacing a channel type from tokio implementation to crossbeam implementation leads to more than 10 KiB size increase. After analyzing the symbols from the final binary, I found symbols from all kinds of flavors, which means at least for me, they are not optimized away.

@taiki-e
Copy link
Member

taiki-e commented Apr 28, 2023

Since opt-level=z has a very low inlining threshold and actively prevents inlining, I would not be surprised if it is not inlined.

Try to see if you have the same problem with lto=fat and opt-level=3. If it works well, we will likely get the same optimization with opt-level=z if we add the appropriate inlining attributes.

@EFanZh
Copy link
Author

EFanZh commented Apr 29, 2023

I have tested the hello world example from the crossbeam-channel document: https://docs.rs/crossbeam-channel/latest/crossbeam_channel/#hello-world with codegen-units = 1 and lto = "fat":

fn main() {
    let (s, r) = crossbeam_channel::unbounded();

    s.send("Hello, world!").unwrap();

    assert_eq!(r.recv(), Ok("Hello, world!"));
}

The result binary contains the following symbols:

$ nm -C target/release/scratch | grep flavor
0000000000007580 t core::ptr::drop_in_place::<core::option::Option<alloc::boxed::Box<crossbeam_channel::flavors::list::Block<&str>>>>
0000000000007590 t core::ptr::drop_in_place::<core::option::Option<<crossbeam_channel::flavors::zero::Channel<&str>>::recv::{closure#1}>>
0000000000007dc0 t core::ptr::drop_in_place::<crossbeam_channel::counter::Counter<crossbeam_channel::flavors::list::Channel<&str>>>
0000000000008030 t <crossbeam_channel::context::Context>::with::<<crossbeam_channel::flavors::list::Channel<&str>>::recv::{closure#1}, ()>::{closure#0} 
0000000000008150 t <crossbeam_channel::context::Context>::with::<<crossbeam_channel::flavors::zero::Channel<&str>>::recv::{closure#1}, core::result::Result<&str, crossbeam_channel::err::RecvTimeoutError>>::{closure#0}
0000000000008770 t <crossbeam_channel::context::Context>::with::<<crossbeam_channel::flavors::array::Channel<&str>>::recv::{closure#1}, ()>::{closure#0}
0000000000009670 t <crossbeam_channel::flavors::zero::Channel<&str>>::disconnect
0000000000009a00 t <alloc::sync::Arc<crossbeam_channel::flavors::at::Channel>>::drop_slow

@taiki-e
Copy link
Member

taiki-e commented Apr 29, 2023

Thanks for the minimal reproduction.

I compiled your example with the following configuration and rustc version,

[dependencies]
crossbeam-channel = "=0.5.8"

[profile.release]
lto = "fat"
codegen-units = 1
$ rustc -vV
rustc 1.71.0-nightly (f49560538 2023-04-28)
binary: rustc
commit-hash: f4956053816439a5884cb2ad1247835858f92218
commit-date: 2023-04-28
host: aarch64-apple-darwin
release: 1.71.0-nightly
LLVM version: 16.0.2

and was able to reproduce the problem (I wonder if it is thanks to LLVM 16 that there are less dead code than what you saw):

$ cargo build -rq && nm -C target/release/repro | grep flavor
0000000100002240 t core::ptr::drop_in_place$LT$crossbeam_channel..counter..Counter$LT$crossbeam_channel..flavors..list..Channel$LT$$RF$str$GT$$GT$$GT$::hd5a62cd6d3dcae89
0000000100002414 t core::ptr::drop_in_place$LT$core..option..Option$LT$alloc..boxed..Box$LT$crossbeam_channel..flavors..list..Block$LT$$RF$str$GT$$GT$$GT$$GT$::h0fec0387e4ed1fca
0000000100002420 t core::ptr::drop_in_place$LT$core..option..Option$LT$crossbeam_channel..flavors..zero..Channel$LT$$RF$str$GT$..recv..$u7b$$u7b$closure$u7d$$u7d$$GT$$GT$::h8a21fd85479524c7

The dead code (flavors..zero) that failed to remove seemed to be from the code that runs during unwinding and can be removed by setting panic=abort. (related to rust-lang/rust#83845)

[profile.release]
lto = "fat"
codegen-units = 1
panic = "abort"
$ cargo build -rq && nm -C target/release/repro | grep flavor
00000001000014cc t core::ptr::drop_in_place$LT$alloc..boxed..Box$LT$crossbeam_channel..counter..Counter$LT$crossbeam_channel..flavors..list..Channel$LT$$RF$str$GT$$GT$$GT$$GT$::hc82e97dda2100084

Even with opt-level=z, it seems that dead code can be removed if lto=fat and panic=abort are enabled.

[profile.release]
lto = "fat"
opt-level = "z"
panic = "abort"
$ cargo build -rq && nm -C target/release/repro | grep flavor
0000000100000c34 t crossbeam_channel::flavors::list::Slot$LT$T$GT$::wait_write::ha7203ac9912aeda3
0000000100000c70 t crossbeam_channel::flavors::list::Block$LT$T$GT$::new::h8a5c350203da28f8
0000000100000cc4 t crossbeam_channel::flavors::list::Block$LT$T$GT$::wait_next::h3164c75c614f016c
0000000100000d04 t core::ptr::drop_in_place$LT$alloc..boxed..Box$LT$crossbeam_channel..flavors..list..Block$LT$$RF$str$GT$$GT$$GT$::h4c6a1d1c57d969b5
0000000100000d18 t core::ptr::drop_in_place$LT$core..option..Option$LT$alloc..boxed..Box$LT$crossbeam_channel..flavors..list..Block$LT$$RF$str$GT$$GT$$GT$$GT$::hb37024cca538f30e

Even with opt-level=z + lto=thin, it seems that dead code can be removed by adding an inline attribute to crossbeam-channel if panic=abort is enabled.

[dependencies]
crossbeam-channel = { version = "0.5", path = "path/to/my/patch/adding/inline" }

[profile.release]
lto = "thin"
opt-level = "z"
panic = "abort"
$ cargo build -rq && nm -C target/release/repro | grep flavor
0000000100008b34 t crossbeam_channel::flavors::list::Slot$LT$T$GT$::wait_write::h93c3bbe32c5297c4 (.llvm.1592442167755279252)
0000000100008b6c t crossbeam_channel::flavors::list::Block$LT$T$GT$::new::hf363a5d3f17e0af4
0000000100008bc4 t crossbeam_channel::flavors::list::Block$LT$T$GT$::destroy::h59f9a68feee5d140 (.llvm.1592442167755279252)
0000000100008c04 t crossbeam_channel::flavors::list::Block$LT$T$GT$::wait_next::h43780353a1c14dad (.llvm.1592442167755279252)
0000000100008c3c t crossbeam_channel::flavors::list::Channel$LT$T$GT$::start_recv::hba56b7496010d9ba
0000000100008d48 t crossbeam_channel::flavors::list::Channel$LT$T$GT$::start_send::h2071de5f979f3534
0000000100008ef0 t crossbeam_channel::flavors::list::Channel$LT$T$GT$::disconnect_senders::he79d56f6ce3d8770
0000000100008fd4 t crossbeam_channel::flavors::list::Channel$LT$T$GT$::disconnect_receivers::h12462b18cb4c2ed6
00000001000090d4 t crossbeam_channel::flavors::list::Channel$LT$T$GT$::read::h1471287362fdaf12
0000000100009164 t crossbeam_channel::flavors::list::Channel$LT$T$GT$::write::hbbf5f571844f5289
0000000100009334 t core::ptr::drop_in_place$LT$alloc..boxed..Box$LT$crossbeam_channel..flavors..list..Block$LT$$RF$str$GT$$GT$$GT$::hbd848b10f0349303 (.llvm.1592442167755279252)
0000000100009340 t core::ptr::drop_in_place$LT$core..option..Option$LT$alloc..boxed..Box$LT$crossbeam_channel..flavors..list..Block$LT$$RF$str$GT$$GT$$GT$$GT$::hafee7583ff38a949
000000010000942c t _$LT$crossbeam_channel..flavors..list..Channel$LT$T$GT$$u20$as$u20$core..ops..drop..Drop$GT$::drop::hccfef0772dde25b2

So, aside from a bug in the compiler optimization regarding panic=unwind (related to rust-lang/rust#83845), I believe this is working as I expected in #979 (comment).

@taiki-e taiki-e removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author label Apr 29, 2023
@EFanZh
Copy link
Author

EFanZh commented Apr 29, 2023

Unfortunately, panic=abort is not really usable in my project, because I need the backtrace information, even on release builds.

@taiki-e
Copy link
Member

taiki-e commented Apr 29, 2023

If you are using panic=unwind only for getting backtrace, it may be possible to use -C force-unwind-tables instead (see also rust-lang/backtrace-rs#397).

@EFanZh
Copy link
Author

EFanZh commented May 16, 2023

I have confirmed that panic=abort is not acceptable in my project. The process needs to continue to run even after a panic happens.

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

No branches or pull requests

2 participants