RickyDaMa
commented
10 months ago Okay, the crate is now a lot more polished and ready for use. However, I've come across an issue with integrating with norad specifically, mainly due to limitations in how you can specify features in Cargo.tomls
Background: close_already now supports multiple different backends to avoid conflicting threadpools where you're already using something like rayon, or an async runtime. These backends are chosen from with mutually exclusive backend-blah features
I presume the rayon dependency/feature isn't enabled in this crate because rayon is relatively heavy as dependencies go, which is fair enough. Per close_already's backend-selection, when we are using rayon, I want to activate the backend-rayon feature, so it doesn't have a separate threadpool 'fighting' with rayon's
That's all well and good, until we want to make close_already default when rayon isn't enabled (given that's where the biggest gains are), but using a lighter backend (the SLoC quote from before was using the now-named backend-threadpool)
An attempt at this setup looks something like this:
[dependencies]
# -- snip --
rayon = { version = "1.3.0", optional = true }
# Note: no backend enabled here
close_already = { version = "0.3", default-features = false }
[features]
# By default, enable the backend-threadpool feature of close_already
default = ["close_already/backend-threadpool"]
# The "rayon" feature enables the dependency rayon and activates the backend-rayon feature
rayon = ["dep:rayon", "close_already/backend-rayon"]The problem now is that building with the rayon feature has two backends enabled, resulting in close_already failing to compile. There is no way in the Cargo.toml to specify "de-activate backend-threadpool when the rayon feature is enabled". (Tangent: mutually exclusive features don't even exist at a Cargo.toml level, you have to detect it at compile time in source code and compile_error!, which is ugly and awkward)
Potential fixes:
- Require users to specify
no-default-features = truewhen they want to usenoradwith the rayon feature - Use
close_alreadyin either no-rayon or rayon mode, but not both. This however then requires#[cfg]flags throughout the code asclose_alreadyisn't always present - Always use
backend-rayon(at that point you could just remove the rayon feature and make it default, sincerayonwill be compiled in anyway) - Change
close_alreadysomehow to resolve which backend you actually want if you enable multiple (how??)
Interested in thoughts, feelings, and emotions from people. If we want more reproducible benchmarks to make a decision based on close_already's value proposition, I'm happy to get something put together for that and share it here
cmyr
Full disclosure, this is basically an advert for a little crate I wrote over the weekend:
close_alreadyIn short, by using my crate (under 1000 SLoC including the two transitive dependencies*) I was able to speed up norad (without rayon) writing a big UFO to disk by 67%, and a further ~10% when the rayon feature was enabled (despite a suboptimal implementation with conflicting threadpools). You can read more about how this works in the crate's README
I still need to benchmark read performance for exact numbers, but there should also be performance improvements there as well
I've made the library work as a no-op on other compile targets and tried to keep it simple & uninvasive to integrate, so I was wondering if
noradwould be interested in a PR to show what this looks like?* EDIT: when using the default
backend-threadpool