search

GitoxideLabs / gitoxide

An idiomatic, lean, fast & safe pure Rust implementation of Git
Apache License 2.0
9.02k stars 311 forks source link

slow compile time, compared to git2 (in a crate without proc macros) #729

Closed benmkw closed 1 year ago

benmkw commented 1 year ago

Duplicates

Current behavior 😯

I have a super small program which opens the github repository of a cloned repo in the browser.

https://github.com/benmkw/githome

After switching from git2 to gitoxide, the compilation time went roughly from 12s to 30s. This seems high to me as everything I need is basically search for a git repo and read a text file. The resulitng binary is also larger with 1.8MB compared to 1.2MB (on macos). (I activated the usual nobs for reducing binary size in Cargo.toml)

Expected behavior 🤔

None of this is a practical problem for me but I wanted to report it because I think that gitoxide could be faster than git2 because it needs to compile less code (its already split into many crates).

Steps to reproduce 🕹

clonse https://github.com/benmkw/githome and compare 0db0221 (gitoxide) to b828bf4 (git2)

Byron commented 1 year ago

Thanks for bringing it up and for providing a way to reproduce the issue.

My quick and unscientific measurements mostly confirm the findings:

    Checking git-refspec v0.7.2
    Checking git-worktree v0.12.2
    Checking git-discover v0.13.0
    Checking git-config v0.16.0
    Checking git-odb v0.40.2
    Checking git-repository v0.35.0
    Checking githome v0.3.0 (/Users/byron/dev/githome)
    Finished dev [unoptimized + debuginfo] target(s) in 5.83s

githome (0db0221) took 5s
❯ gco b828bf4
Previous HEAD position was 0db0221 deps
HEAD is now at b828bf4 up deps and better error message

githome (b828bf4)
❯ cargo clean

githome (b828bf4)
❯ cargo check
   Compiling libc v0.2.76
   Compiling pkg-config v0.3.18
   Compiling autocfg v1.0.1
    Checking tinyvec v0.3.4
    Checking matches v0.1.8
   Compiling log v0.4.11
   Compiling bitflags v1.2.1
    Checking cfg-if v0.1.10
    Checking percent-encoding v2.1.0
    Checking opener v0.4.1
    Checking unicode-bidi v0.3.4
    Checking unicode-normalization v0.1.13
   Compiling jobserver v0.1.21
    Checking idna v0.2.0
   Compiling cc v1.0.59
    Checking url v2.1.1
   Compiling libz-sys v1.1.0
   Compiling openssl-sys v0.9.58
   Compiling libssh2-sys v0.2.19
   Compiling libgit2-sys v0.12.12+1.0.1
    Checking git2 v0.13.10
    Checking githome v0.2.0 (/Users/byron/dev/githome)
    Finished dev [unoptimized + debuginfo] target(s) in 4.23s

githome (b828bf4) took 4s

The folks over at helix did a similar comparison and saw only a 0.5s difference. This might be due to them including proc-macro infrastructure anyway (syn and friends) so they had to pay for these anyway. Here that's not the case so the difference is much larger.

The difference in binary size needs more investigation as to what it is that consumes so much space, even though I wonder how this can be fixed given that Rust already removes everything that it doesn't need (i.e. there is no dead code in binaries).

This issue probably won't be worked on for a long time but it shall serve as a reminder that (especially the binary size) is something to investigate and hopefully fix one day.

benmkw commented 1 year ago

here is the result of rm -rf target && cargo build --release --timings

timings

I'm not sure what to conclude from that, just seems like git-repository actually needs a long time to build (and most of it is codegen if I read that correctly)

regarding the binary size 


cargo bloat --release      

warning: `githome` (bin "githome") generated 1 warning
    Finished release [optimized] target(s) in 0.08s
    Analyzing target/release/githome

 File  .text     Size          Crate Name
 1.1%   2.3%  17.3KiB git_repository git_repository::open::repository::<impl git_repository::types::ThreadSafeRepositor...
 1.1%   2.1%  16.2KiB        githome githome::main
 1.0%   2.0%  15.5KiB git_repository git_repository::config::cache::init::<impl git_repository::config::Cache>::from_st...
 1.0%   2.0%  15.3KiB            std addr2line::ResDwarf<R>::parse
 0.6%   1.3%   9.9KiB           idna idna::uts46::Idna::to_ascii_inner
 0.6%   1.3%   9.6KiB            std std::sys_common::backtrace::_print_fmt::{{closure}}
 0.6%   1.2%   8.9KiB           time time::parsing::parsed::Parsed::parse_component
 0.6%   1.2%   8.8KiB            std addr2line::Context<R>::find_frames
 0.5%   1.0%   7.6KiB            std addr2line::ResUnit<R>::parse_lines
 0.4%   0.9%   6.6KiB git_repository git_repository::repository::remote::<impl git_repository::types::Repository>::find...
 0.4%   0.9%   6.6KiB        git_odb git_odb::store_impls::dynamic::load_index::<impl git_odb::Store>::collect_indices_...
 0.4%   0.8%   6.2KiB            std std::sys::unix::process::process_inner::<impl std::sys::unix::process::process_com...
 0.4%   0.8%   5.7KiB     git_config git_config::file::access::mutate::<impl git_config::types::File>::append_or_insert
 0.4%   0.7%   5.4KiB     git_config git_config::file::includes::resolve_includes_recursive
 0.3%   0.7%   5.0KiB            url url::parser::Parser::after_double_slash
 0.3%   0.6%   4.6KiB            std gimli::read::unit::parse_attribute
 0.3%   0.6%   4.5KiB   git_revision git_revision::spec::parse::function::revision
 0.3%   0.6%   4.5KiB        git_ref git_ref::store_impl::file::packed::<impl git_ref::store_impl::file::Store>::open_p...
 0.3%   0.6%   4.3KiB git_repository git_repository::repository::config::remote::<impl git_repository::types::Repositor...
 0.3%   0.6%   4.2KiB            url url::parser::Parser::parse_path
39.3%  78.3% 595.9KiB                And 1623 smaller methods. Use -n N to show more.
50.1% 100.0% 761.2KiB                .text section size, the file size is 1.5MiB
Byron commented 1 year ago

That's wonderful, I never actually collected data like that but after having seen it, it will be hard not to use the --timings flag each time.

Indeed, codegen takes quite some time, but generally seems to be 70% - 80% of the overall runtime, and longer rustc runtimes seem to increase the codegen runtime proportionally.

In theory, it's possible to add feature toggles for everything which should be reducing dependencies and also codegen time due to less code actually being compiled.

benmkw commented 1 year ago

One thing I was wondering is that the default Repository is threasafe. I'm not sure I fully understand the implications here/ if I need this or not but my guess would be that I don't really need any threadsafety and maybe that could simplify things a bit.

Maybe a not-threadsafe api would be worth exposing for such simple usecases. On the other hand I'm not sure that this would be the largest improvement.

Another usecase I could image is like getting the latest git commit hash, its pretty trivial and does not really need to be optimised but a robust way to do that would still be nice and a small compile/ size cost would be nice here as well ... and turns out gitoxide is already there :D https://github.com/rustyhorde/vergen/pull/112#issuecomment-1363972416

So I did a small measurement here as well, so it seems gitoxide is quite a bit slower even in a larger project there as well:

rm -rf target && cargo build --release --features=gitoxide,git  
    Finished release [optimized] target(s) in 14.22s
    Finished release [optimized] target(s) in 14.14s

rm -rf target && cargo build --release --features=git2,git  
    Finished release [optimized] target(s) in 10.09s
    Finished release [optimized] target(s) in 10.26s

libvergen.rlib(is this the right thing to look at?) is 1.2Mb for --features=gitoxide,git and 690Kb for --features=git2,git

Byron commented 1 year ago

Maybe a not-threadsafe api would be worth exposing for such simple usecases. On the other hand I'm not sure that this would be the largest improvement.

That's actually possible already, even though I am not sure it has any implications to compile times or binary sizes. gix = { version = "x", default-features = false }. That way, it will turn parallelism off entirely.

It would be very interesting to find out what the boundaries are. What if you would remove all unnecessary code to power githome from gix and see what that does to compile times and binary sizes. It would be good to see what's in the cards for these metrics.

Edit: I have a feeling that compiling C code with a C compiler will always be faster and nothing one can reasonably beat at least while LLVM generates the code.

benmkw commented 1 year ago

gix = { version = "x", default-features = false }

I did that already but I think the default gix-repository open still forwards to ThreadSafeRepository::open (but maybe its not actually threadsafe when the features are disabled so thats fine?).

Edit: I have a feeling that compiling C code with a C compiler will always be faster and nothing one can reasonably beat at least while LLVM generates the code.

I recently compiled a 15Mb rust file containing only if-else statements (it was a random forest model) and it was really slow, but it turned out the rust compiler was generating warnings and outputting them was extremely slow (even on a gpu accelerated terminal) and once the warnings were removed, basic codegen was actually pretty fast. So I would not give up hope so fast.

Maybe re-reading https://matklad.github.io/2021/09/04/fast-rust-builds.html could help, I probably use too much generics instead of dyn trait myself and maybe this is also an issue here (just a guess)

Byron commented 1 year ago

(but maybe its not actually threadsafe when the features are disabled so thats fine?).

Indeed, most parts of it aren't actually threadsafe then.

I probably use too much generics instead of dyn trait myself and maybe this is also an issue here (just a guess)

There might be possible to use less generics, even though I intentionally use them plumbing crates to make their usages easier. After all, generics instantiated only once should not affect the code size at all. My main goal here would be to try and see which strategies work (and how much room there is even) without changing the actual code (much).

benmkw commented 1 year ago

good news, build times seem to have improved, its now roughly 11s for git2 vs 20s (about 30s before) for gitoxide now (githome at 136cd15fe87e37380d86aef65e043b08e1ed71cb)

Byron commented 1 year ago

Thanks for the update, that's great news. It seems that ditching dashmap at least contributed to that effect.

benmkw commented 1 year ago

One more related thing to optimise slightly is running cargo tree -d and seeing if a dependency is included twice with different versions. E.g. hashbrown is used twice in my case, currently once by gix-hashtable v0.1.2 -> hashbrown v0.13.2 and once gix-diff v0.28.0 -> imara-diff v0.1.5 -> hashbrown v0.12.3.

Maybe another question to ask is if viewing/ computing diffs is maybe a separate use case and should be enabled by its own feature. Opening/ fetching/ pushing repositories might all be frequent things which don't involve diffs (I could be wrong here though of course) and thus could do without this dependency.

Byron commented 1 year ago

Right, thanks for bringing this up. There is a couple of low-hanging fruit in terms of avoiding dependencies still to be picked. This is planned at the end of the 'gitoxide replaces git2' work, which is tracked here. It's nothing that happens immediately, but I plan to add feature toggles to allow more fine-grained control over which crates are pulled in by gix.

In this specific case, imara-diff has a new breaking release incoming which will certainly update dependencies as well.

Byron commented 1 year ago

Please note that #987 has recently been opened. It's tracking progress in that regard so maybe this issue can be closed in its favour.

benmkw commented 1 year ago

sure, feel free to use https://github.com/benmkw/githome as a way to measure the speedup, other than that, the new issue seems to have much more (actionable) detail so this can be closed

benmkw commented 8 months ago

I just reran rm -rf target && cargo build --release --timings with the lastest versions again

code: https://github.com/benmkw/githome/commit/4fe9573bef60cc5de443b0b722326da4e2363585

timings

Investigating just btoi

cargo tree -i num-traits

num-traits v0.2.18
└── btoi v0.4.3
    ├── gix-actor v0.30.0
    │   ├── gix v0.58.0
    │   │   └── githome v0.3.0 (/Users/benediktmandelkow/gits/my_rust/githome)
    │   ├── gix-object v0.41.0
    │   │   ├── gix v0.58.0 (*)
    │   │   ├── gix-diff v0.40.0
    │   │   │   └── gix v0.58.0 (*)
    │   │   ├── gix-odb v0.57.0
    │   │   │   └── gix v0.58.0 (*)
    │   │   ├── gix-pack v0.47.0
    │   │   │   ├── gix v0.58.0 (*)
    │   │   │   └── gix-odb v0.57.0 (*)
    │   │   ├── gix-ref v0.41.0
    │   │   │   ├── gix v0.58.0 (*)
    │   │   │   ├── gix-config v0.34.0
    │   │   │   │   └── gix v0.58.0 (*)
    │   │   │   └── gix-discover v0.29.0
    │   │   │       ├── githome v0.3.0 (/Users/benediktmandelkow/gits/my_rust/githome)
    │   │   │       └── gix v0.58.0 (*)
    │   │   ├── gix-revision v0.26.0
    │   │   │   ├── gix v0.58.0 (*)
    │   │   │   └── gix-refspec v0.22.0
    │   │   │       └── gix v0.58.0 (*)
    │   │   ├── gix-revwalk v0.12.0
    │   │   │   ├── gix v0.58.0 (*)
    │   │   │   ├── gix-revision v0.26.0 (*)
    │   │   │   └── gix-traverse v0.37.0
    │   │   │       └── gix v0.58.0 (*)
    │   │   └── gix-traverse v0.37.0 (*)
    │   └── gix-ref v0.41.0 (*)
    ├── gix-object v0.41.0 (*)
    └── gix-quote v0.4.10
        └── gix-odb v0.57.0 (*)

I wondered if btoi can be compiled without num-traits, and it seems by deactivating the default feature could be possible.

The situation has improved since my first report from around 25s to 17s 🎉 maybe the remaining 5s to git2 can be achieved as well :)

Byron commented 8 months ago

Thanks for researching this! And also, I am glad that the compile time went down, even though I am not sure if it's due to work I did, or everything else.

numtraits is very slow to compile indeed, but it seems to be mandatory in btoi. It's actually just a couple of lines of code, would you be interested to porting these into the couple of types that gitoxide is actually using? It's probably not more than two: i64 and u64, effectively. Such functionality could go into gix-utils and save a core 2.25s, along with two dependencies less to download.

Of course, we could also wait and hope that over time, the compiler will get faster or specifically optimizes for some cases like these.

[...] maybe the remaining 5s to git2 can be achieved as well :)

Over time, with machines of the future, 5s are probably achievable, and hopefully nobody will have to compile git2 by then anymore because memory safety is a human right or something ;).

benmkw commented 8 months ago

I actually inlined btoi but could not really measure much difference for githome, maybe 0.5s in total.

I pasted it below for anyone to take but I'm not yet sure its worth it.

diff --git a/Cargo.lock b/Cargo.lock
index da80b6cd6..6c1c509b9 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1429,12 +1429,12 @@ name = "gix-actor"
 version = "0.30.0"
 dependencies = [
  "bstr",
- "btoi",
  "document-features",
  "gix-date 0.8.3",
  "gix-features 0.38.0",
  "gix-hash 0.14.1",
  "gix-testtools",
+ "gix-utils 0.1.9",
  "itoa",
  "pretty_assertions",
  "serde",
@@ -2002,7 +2002,6 @@ version = "0.30.0"
 dependencies = [
  "bitflags 2.4.1",
  "bstr",
- "btoi",
  "document-features",
  "filetime",
  "fnv",
@@ -2013,6 +2012,7 @@ dependencies = [
  "gix-lock 13.1.0",
  "gix-object 0.41.0",
  "gix-traverse 0.37.0",
+ "gix-utils 0.1.9",
  "hashbrown 0.14.3",
  "itoa",
  "libc",
@@ -2159,7 +2159,6 @@ name = "gix-object"
 version = "0.41.0"
 dependencies = [
  "bstr",
- "btoi",
  "criterion",
  "document-features",
  "gix-actor 0.30.0",
@@ -2167,6 +2166,7 @@ dependencies = [
  "gix-features 0.38.0",
  "gix-hash 0.14.1",
  "gix-testtools",
+ "gix-utils 0.1.9",
  "gix-validate 0.8.3",
  "itoa",
  "pretty_assertions",
@@ -2347,7 +2347,6 @@ dependencies = [
  "async-std",
  "async-trait",
  "bstr",
- "btoi",
  "document-features",
  "futures-io",
  "futures-lite 2.1.0",
@@ -2358,6 +2357,7 @@ dependencies = [
  "gix-packetline",
  "gix-testtools",
  "gix-transport",
+ "gix-utils 0.1.9",
  "maybe-async",
  "serde",
  "thiserror",
@@ -2380,7 +2380,7 @@ name = "gix-quote"
 version = "0.4.10"
 dependencies = [
  "bstr",
- "btoi",
+ "gix-utils 0.1.9",
  "thiserror",
 ]

diff --git a/gix-actor/Cargo.toml b/gix-actor/Cargo.toml
index 9068bcf45..56afd8f3c 100644
--- a/gix-actor/Cargo.toml
+++ b/gix-actor/Cargo.toml
@@ -19,19 +19,24 @@ serde = ["dep:serde", "bstr/serde", "gix-date/serde"]
 [dependencies]
 gix-features = { version = "^0.38.0", path = "../gix-features", optional = true }
 gix-date = { version = "^0.8.3", path = "../gix-date" }
+gix-utils = { version = "^0.1.9", path = "../gix-utils" }

 thiserror = "1.0.38"
-btoi = "0.4.2"
-bstr = { version = "1.3.0", default-features = false, features = ["std", "unicode"]}
+bstr = { version = "1.3.0", default-features = false, features = [
+    "std",
+    "unicode",
+] }
 winnow = { version = "0.6.0", features = ["simd"] }
 itoa = "1.0.1"
-serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"]}
+serde = { version = "1.0.114", optional = true, default-features = false, features = [
+    "derive",
+] }

 document-features = { version = "0.2.0", optional = true }

 [dev-dependencies]
 pretty_assertions = "1.0.0"
-gix-testtools = { path = "../tests/tools"}
+gix-testtools = { path = "../tests/tools" }
 gix-hash = { path = "../gix-hash" }

 [package.metadata.docs.rs]
diff --git a/gix-actor/src/signature/decode.rs b/gix-actor/src/signature/decode.rs
index 32d0a8351..e316b9026 100644
--- a/gix-actor/src/signature/decode.rs
+++ b/gix-actor/src/signature/decode.rs
@@ -1,7 +1,7 @@
 pub(crate) mod function {
     use bstr::ByteSlice;
-    use btoi::btoi;
     use gix_date::{time::Sign, OffsetInSeconds, SecondsSinceUnixEpoch, Time};
+    use gix_utils::btoi::btoi;
     use winnow::{
         combinator::{alt, separated_pair, terminated},
         error::{AddContext, ParserError, StrContext},
diff --git a/gix-index/Cargo.toml b/gix-index/Cargo.toml
index 472094d4e..fbd712ee2 100644
--- a/gix-index/Cargo.toml
+++ b/gix-index/Cargo.toml
@@ -20,13 +20,17 @@ test = true
 serde = ["dep:serde", "smallvec/serde", "gix-hash/serde"]

 [dependencies]
-gix-features = { version = "^0.38.0", path = "../gix-features", features = ["rustsha1", "progress"] }
+gix-features = { version = "^0.38.0", path = "../gix-features", features = [
+    "rustsha1",
+    "progress",
+] }
 gix-hash = { version = "^0.14.1", path = "../gix-hash" }
 gix-bitmap = { version = "^0.2.10", path = "../gix-bitmap" }
 gix-object = { version = "^0.41.0", path = "../gix-object" }
 gix-traverse = { version = "^0.37.0", path = "../gix-traverse" }
 gix-lock = { version = "^13.0.0", path = "../gix-lock" }
 gix-fs = { version = "^0.10.0", path = "../gix-fs" }
+gix-utils = { version = "^0.1.9", path = "../gix-utils" }

 hashbrown = "0.14.3"
 fnv = "1.0.7"
@@ -35,16 +39,20 @@ memmap2 = "0.9.0"
 filetime = "0.2.15"
 bstr = { version = "1.3.0", default-features = false }

-serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] }
+serde = { version = "1.0.114", optional = true, default-features = false, features = [
+    "derive",
+] }
 smallvec = "1.7.0"
-btoi = "0.4.3"
 itoa = "1.0.3"
 bitflags = "2"

 document-features = { version = "0.2.0", optional = true }

 [target.'cfg(not(windows))'.dependencies]
-rustix = { version = "0.38.20", default-features = false, features = ["std", "fs"] }
+rustix = { version = "0.38.20", default-features = false, features = [
+    "std",
+    "fs",
+] }
 libc = { version = "0.2.149" }

 [package.metadata.docs.rs]
diff --git a/gix-index/src/extension/tree/decode.rs b/gix-index/src/extension/tree/decode.rs
index 530c65d8f..edeabd153 100644
--- a/gix-index/src/extension/tree/decode.rs
+++ b/gix-index/src/extension/tree/decode.rs
@@ -22,10 +22,10 @@ fn one_recursive(data: &[u8], hash_len: usize) -> Option<(Tree, &[u8])> {
     let (path, data) = split_at_byte_exclusive(data, 0)?;

     let (entry_count, data) = split_at_byte_exclusive(data, b' ')?;
-    let num_entries: i32 = btoi::btoi(entry_count).ok()?;
+    let num_entries: i32 = gix_utils::btoi::btoi(entry_count).ok()?;

     let (subtree_count, data) = split_at_byte_exclusive(data, b'\n')?;
-    let subtree_count: usize = btoi::btou(subtree_count).ok()?;
+    let subtree_count: usize = gix_utils::btoi::btou(subtree_count).ok()?;

     let (id, mut data) = if num_entries >= 0 {
         let (hash, data) = split_at_pos(data, hash_len)?;
diff --git a/gix-object/Cargo.toml b/gix-object/Cargo.toml
index c2c3692b6..29b6d19de 100644
--- a/gix-object/Cargo.toml
+++ b/gix-object/Cargo.toml
@@ -20,7 +20,13 @@ path = "./benches/decode_objects.rs"

 [features]
 ## Data structures implement `serde::Serialize` and `serde::Deserialize`.
-serde = ["dep:serde", "bstr/serde", "smallvec/serde", "gix-hash/serde", "gix-actor/serde"]
+serde = [
+    "dep:serde",
+    "bstr/serde",
+    "smallvec/serde",
+    "gix-hash/serde",
+    "gix-actor/serde",
+]
 ## When parsing objects by default errors will only be available on the granularity of success or failure, and with the above flag enabled
 ## details information about the error location will be collected.
 ## Use it in applications which expect broken or invalid objects or for debugging purposes. Incorrectly formatted objects aren't at all
@@ -28,28 +34,35 @@ serde = ["dep:serde", "bstr/serde", "smallvec/serde", "gix-hash/serde", "gix-act
 verbose-object-parsing-errors = ["winnow/std"]

 [dependencies]
-gix-features = { version = "^0.38.0", path = "../gix-features", features = ["rustsha1", "progress"] }
+gix-features = { version = "^0.38.0", path = "../gix-features", features = [
+    "rustsha1",
+    "progress",
+] }
 gix-hash = { version = "^0.14.1", path = "../gix-hash" }
 gix-validate = { version = "^0.8.3", path = "../gix-validate" }
 gix-actor = { version = "^0.30.0", path = "../gix-actor" }
 gix-date = { version = "^0.8.3", path = "../gix-date" }
+gix-utils = { version = "^0.1.9", path = "../gix-utils" }

-btoi = "0.4.2"
 itoa = "1.0.1"
 thiserror = "1.0.34"
-bstr = { version = "1.3.0", default-features = false, features = ["std", "unicode"] }
+bstr = { version = "1.3.0", default-features = false, features = [
+    "std",
+    "unicode",
+] }
 winnow = { version = "0.6.0", features = ["simd"] }
 smallvec = { version = "1.4.0", features = ["write"] }
-serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"]}
+serde = { version = "1.0.114", optional = true, default-features = false, features = [
+    "derive",
+] }

 document-features = { version = "0.2.0", optional = true }

 [dev-dependencies]
 criterion = "0.5.1"
 pretty_assertions = "1.0.0"
-gix-testtools = { path = "../tests/tools"}
+gix-testtools = { path = "../tests/tools" }

 [package.metadata.docs.rs]
 all-features = true
 features = ["document-features"]
-
diff --git a/gix-object/src/lib.rs b/gix-object/src/lib.rs
index 85ff8708e..57e02e253 100644
--- a/gix-object/src/lib.rs
+++ b/gix-object/src/lib.rs
@@ -358,7 +358,7 @@ pub mod decode {
     pub enum LooseHeaderDecodeError {
         #[error("{message}: {number:?}")]
         ParseIntegerError {
-            source: btoi::ParseIntegerError,
+            source: gix_utils::btoi::ParseIntegerError,
             message: &'static str,
             number: bstr::BString,
         },
@@ -383,7 +383,7 @@ pub mod decode {
             message: "Did not find 0 byte in header",
         })?;
         let size_bytes = &input[kind_end + 1..size_end];
-        let size = btoi::btoi(size_bytes).map_err(|source| ParseIntegerError {
+        let size = gix_utils::btoi::btoi(size_bytes).map_err(|source| ParseIntegerError {
             source,
             message: "Object size in header could not be parsed",
             number: size_bytes.into(),
diff --git a/gix-protocol/Cargo.toml b/gix-protocol/Cargo.toml
index 1713c6401..0131463af 100644
--- a/gix-protocol/Cargo.toml
+++ b/gix-protocol/Cargo.toml
@@ -23,7 +23,12 @@ doctest = false
 ## If set, blocking command implementations are available and will use the blocking version of the `gix-transport` crate.
 blocking-client = ["gix-transport/blocking-client", "maybe-async/is_sync"]
 ## As above, but provides async implementations instead.
-async-client = ["gix-transport/async-client", "async-trait", "futures-io", "futures-lite"]
+async-client = [
+    "gix-transport/async-client",
+    "async-trait",
+    "futures-io",
+    "futures-lite",
+]

 #! ### Other
 ## Data structures implement `serde::Serialize` and `serde::Deserialize`.
@@ -40,17 +45,24 @@ path = "tests/async-protocol.rs"
 required-features = ["async-client"]

 [dependencies]
-gix-features = { version = "^0.38.0", path = "../gix-features", features = ["progress"] }
+gix-features = { version = "^0.38.0", path = "../gix-features", features = [
+    "progress",
+] }
 gix-transport = { version = "^0.41.0", path = "../gix-transport" }
 gix-hash = { version = "^0.14.1", path = "../gix-hash" }
 gix-date = { version = "^0.8.3", path = "../gix-date" }
 gix-credentials = { version = "^0.24.0", path = "../gix-credentials" }
+gix-utils = { version = "^0.1.9", path = "../gix-utils" }

 thiserror = "1.0.32"
-serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"]}
-bstr = { version = "1.3.0", default-features = false, features = ["std", "unicode"] }
+serde = { version = "1.0.114", optional = true, default-features = false, features = [
+    "derive",
+] }
+bstr = { version = "1.3.0", default-features = false, features = [
+    "std",
+    "unicode",
+] }
 winnow = { version = "0.6.0", features = ["simd"] }
-btoi = "0.4.2"

 # for async-client
 async-trait = { version = "0.1.51", optional = true }
@@ -62,7 +74,7 @@ document-features = { version = "0.2.0", optional = true }

 [dev-dependencies]
 async-std = { version = "1.9.0", features = ["attributes"] }
-gix-packetline = { path = "../gix-packetline" ,version = "^0.17.3" }
+gix-packetline = { path = "../gix-packetline", version = "^0.17.3" }
 gix-testtools = { path = "../tests/tools" }

 [package.metadata.docs.rs]
diff --git a/gix-protocol/src/remote_progress.rs b/gix-protocol/src/remote_progress.rs
index 2e066b618..fa6db1d86 100644
--- a/gix-protocol/src/remote_progress.rs
+++ b/gix-protocol/src/remote_progress.rs
@@ -75,7 +75,7 @@ impl<'a> RemoteProgress<'a> {

 fn parse_number(i: &mut &[u8]) -> PResult<usize, ()> {
     take_till(0.., |c: u8| !c.is_ascii_digit())
-        .try_map(btoi::btoi)
+        .try_map(gix_utils::btoi::btoi)
         .parse_next(i)
 }

diff --git a/gix-quote/Cargo.toml b/gix-quote/Cargo.toml
index f2e808680..050f13a6d 100644
--- a/gix-quote/Cargo.toml
+++ b/gix-quote/Cargo.toml
@@ -13,6 +13,7 @@ include = ["src/**/*", "LICENSE-*"]
 doctest = false

 [dependencies]
-bstr = { version = "1.3.0", default-features = false, features = ["std"]}
+gix-utils = { version = "^0.1.9", path = "../gix-utils" }
+
+bstr = { version = "1.3.0", default-features = false, features = ["std"] }
 thiserror = "1.0.38"
-btoi = "0.4.2"
diff --git a/gix-quote/src/ansi_c.rs b/gix-quote/src/ansi_c.rs
index 40d8db8d1..b95e5fb9d 100644
--- a/gix-quote/src/ansi_c.rs
+++ b/gix-quote/src/ansi_c.rs
@@ -89,7 +89,8 @@ pub fn undo(input: &BStr) -> Result<(Cow<'_, BStr>, usize), undo::Error> {
                                     })?
                                     .read_exact(&mut buf[1..])
                                     .expect("impossible to fail as numbers match");
-                                let byte = btoi::btou_radix(&buf, 8).map_err(|e| undo::Error::new(e, original))?;
+                                let byte =
+                                    gix_utils::btoi::btou_radix(&buf, 8).map_err(|e| undo::Error::new(e, original))?;
                                 out.push(byte);
                                 input = &input[2..];
                                 consumed += 2;
diff --git a/gix-utils/src/btoi.rs b/gix-utils/src/btoi.rs
new file mode 100644
index 000000000..06d8d9247
--- /dev/null
+++ b/gix-utils/src/btoi.rs
@@ -0,0 +1,392 @@
+// ported from https://github.com/niklasf/rust-btoi version 0.4.3
+// see https://github.com/Byron/gitoxide/issues/729#issuecomment-1941515655
+
+/// An error that can occur when parsing an integer.
+///
+/// * No digits
+/// * Invalid digit
+/// * Overflow
+/// * Underflow
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct ParseIntegerError {
+    kind: ErrorKind,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+enum ErrorKind {
+    Empty,
+    InvalidDigit,
+    Overflow,
+    Underflow,
+}
+
+impl ParseIntegerError {
+    fn desc(&self) -> &str {
+        match self.kind {
+            ErrorKind::Empty => "cannot parse integer without digits",
+            ErrorKind::InvalidDigit => "invalid digit found in slice",
+            ErrorKind::Overflow => "number too large to fit in target type",
+            ErrorKind::Underflow => "number too small to fit in target type",
+        }
+    }
+}
+
+impl std::fmt::Display for ParseIntegerError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.desc().fmt(f)
+    }
+}
+
+impl std::error::Error for ParseIntegerError {
+    fn description(&self) -> &str {
+        self.desc()
+    }
+}
+/// minimal subset of traits used by btoi_radix and btou_radix
+pub trait MinNumTraits: Sized + Copy {
+    ///
+    fn from_u32(n: u32) -> Option<Self>;
+    ///
+    fn zero() -> Self;
+    ///
+    fn checked_mul(self, v: Self) -> Option<Self>;
+    ///
+    fn checked_add(self, v: Self) -> Option<Self>;
+    ///
+    fn checked_sub(self, v: Self) -> Option<Self>;
+}
+
+impl MinNumTraits for i64 {
+    fn from_u32(n: u32) -> Option<i64> {
+        Some(n.into())
+    }
+
+    fn zero() -> Self {
+        0
+    }
+
+    fn checked_mul(self, v: i64) -> Option<i64> {
+        i64::checked_mul(self, v)
+    }
+
+    fn checked_add(self, v: i64) -> Option<i64> {
+        i64::checked_add(self, v)
+    }
+
+    fn checked_sub(self, v: i64) -> Option<i64> {
+        i64::checked_sub(self, v)
+    }
+}
+
+impl MinNumTraits for u64 {
+    fn from_u32(n: u32) -> Option<u64> {
+        Some(n.into())
+    }
+
+    fn zero() -> Self {
+        0
+    }
+
+    fn checked_mul(self, v: u64) -> Option<u64> {
+        u64::checked_mul(self, v)
+    }
+
+    fn checked_add(self, v: u64) -> Option<u64> {
+        u64::checked_add(self, v)
+    }
+
+    fn checked_sub(self, v: u64) -> Option<u64> {
+        u64::checked_sub(self, v)
+    }
+}
+
+impl MinNumTraits for usize {
+    fn from_u32(n: u32) -> Option<usize> {
+        n.try_into().ok()
+    }
+
+    fn zero() -> Self {
+        0
+    }
+
+    fn checked_mul(self, v: usize) -> Option<usize> {
+        usize::checked_mul(self, v)
+    }
+
+    fn checked_add(self, v: usize) -> Option<usize> {
+        usize::checked_add(self, v)
+    }
+
+    fn checked_sub(self, v: usize) -> Option<usize> {
+        usize::checked_sub(self, v)
+    }
+}
+
+impl MinNumTraits for i32 {
+    fn from_u32(n: u32) -> Option<Self> {
+        n.try_into().ok()
+    }
+
+    fn zero() -> Self {
+        0
+    }
+
+    fn checked_mul(self, v: i32) -> Option<i32> {
+        i32::checked_mul(self, v)
+    }
+
+    fn checked_add(self, v: i32) -> Option<i32> {
+        i32::checked_add(self, v)
+    }
+
+    fn checked_sub(self, v: i32) -> Option<i32> {
+        i32::checked_sub(self, v)
+    }
+}
+
+impl MinNumTraits for u8 {
+    fn from_u32(n: u32) -> Option<Self> {
+        n.try_into().ok()
+    }
+
+    fn zero() -> Self {
+        0
+    }
+
+    fn checked_mul(self, v: u8) -> Option<u8> {
+        u8::checked_mul(self, v)
+    }
+
+    fn checked_add(self, v: u8) -> Option<u8> {
+        u8::checked_add(self, v)
+    }
+
+    fn checked_sub(self, v: u8) -> Option<u8> {
+        u8::checked_sub(self, v)
+    }
+}
+
+/// Converts a byte slice to an integer. Signs are not allowed.
+///
+/// # Errors
+///
+/// Returns [`ParseIntegerError`] for any of the following conditions:
+///
+/// * `bytes` is empty
+/// * not all characters of `bytes` are `0-9`
+/// * the number overflows `I`
+///
+/// # Panics
+///
+/// Panics in the pathological case that there is no representation of `10`
+/// in `I`.
+///
+/// # Examples
+///
+/// ```
+/// # use btoi::btou;
+/// assert_eq!(Ok(12345), btou(b"12345"));
+/// assert!(btou::<u8>(b"+1").is_err()); // only btoi allows signs
+/// assert!(btou::<u8>(b"256").is_err()); // overflow
+/// ```
+///
+/// [`ParseIntegerError`]: struct.ParseIntegerError.html
+#[track_caller]
+pub fn btou<I: MinNumTraits>(bytes: &[u8]) -> Result<I, ParseIntegerError> {
+    btou_radix(bytes, 10)
+}
+
+/// Converts a byte slice in a given base to an integer. Signs are not allowed.
+///
+/// # Errors
+///
+/// Returns [`ParseIntegerError`] for any of the following conditions:
+///
+/// * `bytes` is empty
+/// * not all characters of `bytes` are `0-9`, `a-z` or `A-Z`
+/// * not all characters refer to digits in the given `radix`
+/// * the number overflows `I`
+///
+/// # Panics
+///
+/// Panics if `radix` is not in the range `2..=36` (or in the pathological
+/// case that there is no representation of `radix` in `I`).
+///
+/// # Examples
+///
+/// ```
+/// # use btoi::btou_radix;
+/// assert_eq!(Ok(255), btou_radix(b"ff", 16));
+/// assert_eq!(Ok(42), btou_radix(b"101010", 2));
+/// ```
+///
+/// [`ParseIntegerError`]: struct.ParseIntegerError.html
+pub fn btou_radix<I: MinNumTraits>(bytes: &[u8], radix: u32) -> Result<I, ParseIntegerError> {
+    assert!(
+        (2..=36).contains(&radix),
+        "radix must lie in the range 2..=36, found {}",
+        radix
+    );
+
+    let base = I::from_u32(radix).expect("radix can be represented as integer");
+
+    if bytes.is_empty() {
+        return Err(ParseIntegerError { kind: ErrorKind::Empty });
+    }
+
+    let mut result = I::zero();
+
+    for &digit in bytes {
+        let x = match char::from(digit).to_digit(radix).and_then(I::from_u32) {
+            Some(x) => x,
+            None => {
+                return Err(ParseIntegerError {
+                    kind: ErrorKind::InvalidDigit,
+                })
+            }
+        };
+        result = match result.checked_mul(base) {
+            Some(result) => result,
+            None => {
+                return Err(ParseIntegerError {
+                    kind: ErrorKind::Overflow,
+                })
+            }
+        };
+        result = match result.checked_add(x) {
+            Some(result) => result,
+            None => {
+                return Err(ParseIntegerError {
+                    kind: ErrorKind::Overflow,
+                })
+            }
+        };
+    }
+
+    Ok(result)
+}
+
+/// Converts a byte slice to an integer.
+///
+/// Like [`btou`], but numbers may optionally start with a sign (`-` or `+`).
+///
+/// # Errors
+///
+/// Returns [`ParseIntegerError`] for any of the following conditions:
+///
+/// * `bytes` has no digits
+/// * not all characters of `bytes` are `0-9`, excluding an optional leading
+///   sign
+/// * the number overflows or underflows `I`
+///
+/// # Panics
+///
+/// Panics in the pathological case that there is no representation of `10`
+/// in `I`.
+///
+/// # Examples
+///
+/// ```
+/// # use btoi::btoi;
+/// assert_eq!(Ok(123), btoi(b"123"));
+/// assert_eq!(Ok(123), btoi(b"+123"));
+/// assert_eq!(Ok(-123), btoi(b"-123"));
+///
+/// assert!(btoi::<i16>(b"123456789").is_err()); // overflow
+/// assert!(btoi::<u32>(b"-1").is_err()); // underflow
+///
+/// assert!(btoi::<i32>(b" 42").is_err()); // leading space
+/// ```
+///
+/// [`btou`]: fn.btou.html
+/// [`ParseIntegerError`]: struct.ParseIntegerError.html
+pub fn btoi<I: MinNumTraits>(bytes: &[u8]) -> Result<I, ParseIntegerError> {
+    btoi_radix(bytes, 10)
+}
+
+/// Converts a byte slice in a given base to an integer.
+///
+/// Like [`btou_radix`], but numbers may optionally start with a sign
+/// (`-` or `+`).
+///
+/// # Errors
+///
+/// Returns [`ParseIntegerError`] for any of the following conditions:
+///
+/// * `bytes` has no digits
+/// * not all characters of `bytes` are `0-9`, `a-z`, `A-Z`, exluding an
+///   optional leading sign
+/// * not all characters refer to digits in the given `radix`, exluding an
+///   optional leading sign
+/// * the number overflows or underflows `I`
+///
+/// # Panics
+///
+/// Panics if `radix` is not in the range `2..=36` (or in the pathological
+/// case that there is no representation of `radix` in `I`).
+///
+/// # Examples
+///
+/// ```
+/// # use btoi::btoi_radix;
+/// assert_eq!(Ok(10), btoi_radix(b"a", 16));
+/// assert_eq!(Ok(10), btoi_radix(b"+a", 16));
+/// assert_eq!(Ok(-42), btoi_radix(b"-101010", 2));
+/// ```
+///
+/// [`btou_radix`]: fn.btou_radix.html
+/// [`ParseIntegerError`]: struct.ParseIntegerError.html
+fn btoi_radix<I: MinNumTraits>(bytes: &[u8], radix: u32) -> Result<I, ParseIntegerError> {
+    assert!(
+        (2..=36).contains(&radix),
+        "radix must lie in the range 2..=36, found {}",
+        radix
+    );
+
+    let base = I::from_u32(radix).expect("radix can be represented as integer");
+
+    if bytes.is_empty() {
+        return Err(ParseIntegerError { kind: ErrorKind::Empty });
+    }
+
+    let digits = match bytes[0] {
+        b'+' => return btou_radix(&bytes[1..], radix),
+        b'-' => &bytes[1..],
+        _ => return btou_radix(bytes, radix),
+    };
+
+    if digits.is_empty() {
+        return Err(ParseIntegerError { kind: ErrorKind::Empty });
+    }
+
+    let mut result = I::zero();
+
+    for &digit in digits {
+        let x = match char::from(digit).to_digit(radix).and_then(I::from_u32) {
+            Some(x) => x,
+            None => {
+                return Err(ParseIntegerError {
+                    kind: ErrorKind::InvalidDigit,
+                })
+            }
+        };
+        result = match result.checked_mul(base) {
+            Some(result) => result,
+            None => {
+                return Err(ParseIntegerError {
+                    kind: ErrorKind::Underflow,
+                })
+            }
+        };
+        result = match result.checked_sub(x) {
+            Some(result) => result,
+            None => {
+                return Err(ParseIntegerError {
+                    kind: ErrorKind::Underflow,
+                })
+            }
+        };
+    }
+
+    Ok(result)
+}
diff --git a/gix-utils/src/lib.rs b/gix-utils/src/lib.rs
index 2e4b3e35f..5e2e32d13 100644
--- a/gix-utils/src/lib.rs
+++ b/gix-utils/src/lib.rs
@@ -13,6 +13,9 @@ pub mod buffers;
 ///
 pub mod str;

+///
+pub mod btoi;
+
 /// A utility to do buffer-swapping with.
 ///
 /// Use `src` to read from and `dest` to write to, and after actually changing data, call [Buffers::swap()].
Byron commented 8 months ago

I think having more than 2.25s shaved off from the compile time is worth it. Maybe you could open a PR to retain authorship?

Otherwise I would create the commit and for lack of options mention you as Co-Author (only). License-wise I think nothing special has to be done, and you already mentioned the original source which should be enough.

benmkw commented 8 months ago

okay nice, thanks for trying it out, I can send a PR tomorrow and we can sort out any remaining remarks.