-
-
Notifications
You must be signed in to change notification settings - Fork 314
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
slow compile time, compared to git2 (in a crate without proc macros) #729
Comments
Thanks for bringing it up and for providing a way to reproduce the issue. My quick and unscientific measurements mostly confirm the findings:
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 ( 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. |
here is the result of 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
|
That's wonderful, I never actually collected data like that but after having seen it, it will be hard not to use the Indeed, codegen takes quite some time, but generally seems to be 70% - 80% of the overall runtime, and longer 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. |
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 rustyhorde/vergen#112 (comment) 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:
|
That's actually possible already, even though I am not sure it has any implications to compile times or binary sizes. It would be very interesting to find out what the boundaries are. What if you would remove all unnecessary code to power 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 did that already but I think the default gix-repository open still forwards to
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) |
Indeed, most parts of it aren't actually threadsafe then.
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). |
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) |
Thanks for the update, that's great news. It seems that ditching |
One more related thing to optimise slightly is running 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. |
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 ' In this specific case, |
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. |
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 |
I just reran code: benmkw/githome@4fe9573 Investigating just btoi
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 :) |
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.
Of course, we could also wait and hope that over time, the compiler will get faster or specifically optimizes for some cases like these.
Over time, with machines of the future, 5s are probably achievable, and hopefully nobody will have to compile |
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()].
|
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. |
okay nice, thanks for trying it out, I can send a PR tomorrow and we can sort out any remaining remarks. |
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)
The text was updated successfully, but these errors were encountered: