Morganamilo / paru

Feature packed AUR helper
GNU General Public License v3.0
5.83k stars 223 forks source link

Removing cargo deps #613

Open Morganamilo opened 2 years ago

Morganamilo commented 2 years ago

There's a couple of deps that could be removed eventually which should help with compile time + binary size:

htmlescape = "0.3.1"                                             # aur rpc should get comment support eventually
kuchiki = "0.8.1"                                                # aur rpc should get comment support eventually                          
once_cell = "1.8.0"                                              # should be stable in std eventually
rss = { version = "2.0.0", default-features = false }            # defer to a binary to get the news, are there any good ones?

Aditionally reqwest/tokio could be dropped with a little bit of headache

reqwest = { version = "0.11.6", features = ["gzip", "socks"] }   # could use curl instead but would lose async support
tokio = { version = "1.14.0", features = ["process"] }           # would lose async support
futures = "0.3.18"                                               # not needed if no tokio

Curl could be used instead of reqwest. This is good as pacman already deps curl. One issue is we may lose async web requests which we currently make use of to do some requests in parallel (mostly devel checks + update checks). There also appears to be a tokio async provider but that doesn't seem too popular so I wouldn't want to reply on it.

Sync curl would let us also drop tokio and the whole async stack but we would have to use a threadpool for parallel requests. Realisitcly we're probably never doing more than 30 requests. Spawning 30 threads probably is not that bad.

leshow commented 2 years ago

Stumbled on the project, chiming in as I've worked with tokio a bunch. I don't think this is an application that really benefits from it so removing it is probably a good idea. On some systems you're going to end up spawning close to 30 threads anyway because the runtime is instantiated with:

#[tokio::main]
async fn main() {}

This will spawn 1 thread per logical CPU for the scheduler & an additional threadpool for synchronous tasks (stuff you'd run in spawn_blocking). So you're likely spawning a bunch of threads that aren't used anyway. Overall, unless there's something I'm missing, you're not getting much benefit from tokio and things may even be faster without it.

For alternatives, if you don't want to use curl you can also go with something like ureq too.

Morganamilo commented 2 years ago

Paru doesn't use any spwawn's or spawn blocking. The only real use for async is selecting and joining network reqests.

leshow commented 2 years ago

Right, sounds like even more reason to switch then! It would likely enable the removal of async_trait too?

Morganamilo commented 2 years ago

Not sure if that's more of a reson. Right now it's all multi layerd. The join is ran on a futures that use select internally.

Also async_trait is only used for tests so I don't really count it as a dep currently.

rtkay123 commented 2 years ago

Not sure if that's more of a reson. Right now it's all multi layerd. The join is ran on a futures that use select internally.

Also async_trait is only used for tests so I don't really count it as a dep currently.

If we have more dependencies like this that aren't used when compiling a package for building, we could move them to [dev-dependencies] section

Morganamilo commented 2 years ago

Mock is it's on feature so the dependency is behind that. The feature gates could possibly be changed to cfg(test) and moving async_trait to dev deps. Not sure if that's better though.

rtkay123 commented 2 years ago

I don't think that alone will make much of a noticeable difference, but at least it's a start. I'll check if there's anything else we can do

Morganamilo commented 2 years ago

Changing it would just be a semantic change. The mock feature is not enabled outside of testing so the dep is not pulled in anyway.

WhyNotHugo commented 1 year ago

For once_cell, this issue tracks upstream progress: https://github.com/rust-lang/rust/issues/74465

Some brief testing indicates that the switch is trivial, but will only work on nightly for now:

diff --git a/src/exec.rs b/src/exec.rs
index 1898d19..9c2702c 100644
--- a/src/exec.rs
+++ b/src/exec.rs
@@ -11,12 +11,12 @@ use std::thread;
 use std::time::Duration;

 use anyhow::{bail, Context, Result};
-use once_cell::sync::Lazy;
+use std::sync::LazyLock;
 use signal_hook::consts::signal::*;
 use signal_hook::flag as signal_flag;
 use tr::tr;

-pub static DEFAULT_SIGNALS: Lazy<Arc<AtomicBool>> = Lazy::new(|| {
+pub static DEFAULT_SIGNALS: LazyLock<Arc<AtomicBool>> = LazyLock::new(|| {
     let arc = Arc::new(AtomicBool::new(true));
     signal_flag::register_conditional_default(SIGTERM, Arc::clone(&arc)).unwrap();
     signal_flag::register_conditional_default(SIGINT, Arc::clone(&arc)).unwrap();
@@ -24,7 +24,7 @@ pub static DEFAULT_SIGNALS: Lazy<Arc<AtomicBool>> = Lazy::new(|| {
     arc
 });

-static CAUGHT_SIGNAL: Lazy<Arc<AtomicUsize>> = Lazy::new(|| {
+static CAUGHT_SIGNAL: LazyLock<Arc<AtomicUsize>> = LazyLock::new(|| {
     let arc = Arc::new(AtomicUsize::new(0));
     signal_flag::register_usize(SIGTERM, Arc::clone(&arc), SIGTERM as usize).unwrap();
     signal_flag::register_usize(SIGINT, Arc::clone(&arc), SIGINT as usize).unwrap();
@@ -32,7 +32,7 @@ static CAUGHT_SIGNAL: Lazy<Arc<AtomicUsize>> = Lazy::new(|| {
     arc
 });

-pub static RAISE_SIGPIPE: Lazy<Arc<AtomicBool>> = Lazy::new(|| {
+pub static RAISE_SIGPIPE: LazyLock<Arc<AtomicBool>> = LazyLock::new(|| {
     let arc = Arc::new(AtomicBool::new(true));
     signal_flag::register_conditional_default(SIGPIPE, Arc::clone(&arc)).unwrap();
     arc
diff --git a/src/lib.rs b/src/lib.rs
index c49be3b..062308e 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1,3 +1,5 @@
+#![feature(once_cell)]
+
 mod args;
 mod chroot;
 mod clean;