Skip to content

Commit 1a92ad2

Browse files
codexByron
authored andcommitted
Remove kstring from gix-attributes.
Add Criterion benchmarks for attribute lookup in a single file with 1, 10, 100, and 1000 entries, plus five nested files with 1024, 512, 256, 128, and 64 entries. The kstring baseline measured 58.393 ns, 670.48 ns, 8.0138 us, and 96.207 us for the single-file cases, and 209.72 us for the hierarchy. Store owned attribute names in gix-features OwnShared<str>, using Rc without the parallel feature and Arc with it, so lookup clones remain allocation-free without another string dependency or leaked storage. Default-mode medians are 55.115 ns, 643.63 ns, 7.9572 us, 95.431 us, and 207.61 us; parallel-mode medians are 57.324 ns, 668.03 ns, 7.8143 us, 94.853 us, and 204.28 us. Outcome shrinks from 840 to 752 bytes. Keep non-parallel consumers Rc-compatible: own the synthetic match-all pathspec per Search, defer CLI pathspec construction until after the worker boundary, transport attribute baseline assignments as thread-safe strings, and cache test exclusions per thread for the repository discovered from each archive path. Forward the parallel opt-in through public dependent crates, with Send assertions for pathspec Search and filter Pipeline. The always-threaded worktree stream enables parallel explicitly. Validation: - just check - cargo +1.85.0 test -p gix-attributes --lib --tests - cargo +1.85.0 test -p gix-attributes --lib --tests --features parallel - cargo test -p gix-attributes - cargo test -p gix-attributes --all-features --lib --tests - cargo test -p gix-pathspec --features parallel - cargo test -p gix-filter --features sha1,parallel - cargo test -p gix-testtools --lib --tests - cargo test -p gitoxide-core --lib - cargo test --no-default-features --features small --lib - cargo clippy -p gix-attributes --all-targets --all-features -- -D warnings - cargo clippy -p gix-pathspec --all-targets --features parallel -- -D warnings - cargo clippy -p gix-filter --all-targets --features sha1,parallel -- -D warnings - cargo clippy --no-default-features --features small -- -D warnings - cargo check --workspace --all-targets - cargo build -p gix-pathspec --target wasm32-wasip1 - cargo build -p gix-pathspec --target wasm32-wasip2 - cargo build -p gix-pathspec --target wasm32-unknown-unknown - cargo bench -p gix-attributes --bench lookup -- --noplot - cargo bench -p gix-attributes --bench lookup --features parallel -- --noplot
1 parent 299d16b commit 1a92ad2

28 files changed

Lines changed: 460 additions & 178 deletions

File tree

Cargo.lock

Lines changed: 42 additions & 18 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

gitoxide-core/src/repository/attributes/validate_baseline.rs

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@ pub(crate) mod function {
1818
};
1919

2020
use anyhow::{anyhow, bail};
21-
use gix::{Count, Progress, attrs::Assignment, bstr::BString};
21+
use gix::{
22+
Count, Progress,
23+
attrs::{Assignment, NameRef},
24+
bstr::{BString, ByteSlice},
25+
};
2226

2327
use crate::{
2428
OutputFormat,
@@ -200,19 +204,19 @@ pub(crate) mod function {
200204
let fast_path_mismatch = matches
201205
.iter()
202206
.map(|m| m.assignment)
203-
.zip(expected.iter().map(Assignment::as_ref))
207+
.zip(expected.iter().map(ThreadSafeAssignment::as_ref))
204208
.any(|(a, b)| a != b);
205209
if fast_path_mismatch {
206210
let actual_set = BTreeSet::from_iter(matches.iter().map(|m| m.assignment));
207-
let expected_set = BTreeSet::from_iter(expected.iter().map(Assignment::as_ref));
211+
let expected_set = BTreeSet::from_iter(expected.iter().map(ThreadSafeAssignment::as_ref));
208212
let too_few_or_too_many =
209213
!(expected_set.sub(&actual_set).is_empty() && actual_set.sub(&expected_set).is_empty());
210214
if too_few_or_too_many {
211215
mismatches.push((
212216
rela_path,
213217
Mismatch::Attributes {
214218
actual: matches.iter().map(|m| m.assignment.to_owned()).collect(),
215-
expected,
219+
expected: expected.into_iter().map(Into::into).collect(),
216220
},
217221
));
218222
}
@@ -256,10 +260,36 @@ pub(crate) mod function {
256260
}
257261

258262
enum Baseline {
259-
Attribute { assignments: Vec<gix::attrs::Assignment> },
263+
Attribute { assignments: Vec<ThreadSafeAssignment> },
260264
Exclude { location: Option<ExcludeLocation> },
261265
}
262266

267+
struct ThreadSafeAssignment {
268+
name: String,
269+
state: gix::attrs::State,
270+
}
271+
272+
impl ThreadSafeAssignment {
273+
fn as_ref(&self) -> gix::attrs::AssignmentRef<'_> {
274+
gix::attrs::AssignmentRef {
275+
name: NameRef::try_from(self.name.as_bytes().as_bstr())
276+
.expect("names from git check-attr were validated while parsing"),
277+
state: self.state.as_ref(),
278+
}
279+
}
280+
}
281+
282+
impl From<ThreadSafeAssignment> for Assignment {
283+
fn from(value: ThreadSafeAssignment) -> Self {
284+
Assignment {
285+
name: NameRef::try_from(value.name.as_bytes().as_bstr())
286+
.expect("names from git check-attr were validated while parsing")
287+
.to_owned(),
288+
state: value.state,
289+
}
290+
}
291+
}
292+
263293
#[derive(Debug)]
264294
// See note on `Mismatch`
265295
#[allow(dead_code)]
@@ -329,7 +359,7 @@ pub(crate) mod function {
329359
let (path, assignment) = parse_attribute_line(&first)?;
330360

331361
let current = path.to_owned();
332-
out.push(assignment.to_owned());
362+
out.push(assignment);
333363
loop {
334364
let next_line = match lines.peek() {
335365
None => break,
@@ -339,15 +369,15 @@ pub(crate) mod function {
339369
if next_path != current {
340370
return Some((current, Baseline::Attribute { assignments: out }));
341371
} else {
342-
out.push(next_assignment.to_owned());
372+
out.push(next_assignment);
343373
lines.next();
344374
}
345375
}
346376
Some((current, Baseline::Attribute { assignments: out }))
347377
}
348378

349-
fn parse_attribute_line(line: &str) -> Option<(&str, gix::attrs::AssignmentRef<'_>)> {
350-
use gix::{attrs::StateRef, bstr::ByteSlice};
379+
fn parse_attribute_line(line: &str) -> Option<(&str, ThreadSafeAssignment)> {
380+
use gix::attrs::StateRef;
351381

352382
let mut prev = None;
353383
let mut tokens = line.splitn(3, |b| {
@@ -364,9 +394,9 @@ pub(crate) mod function {
364394
};
365395
path = path.trim_end_matches(':');
366396
let attr = attr.trim_end_matches(':');
367-
let assignment = gix::attrs::AssignmentRef {
368-
name: gix::attrs::NameRef::try_from(attr.as_bytes().as_bstr()).ok()?,
369-
state,
397+
let assignment = ThreadSafeAssignment {
398+
name: NameRef::try_from(attr.as_bytes().as_bstr()).ok()?.as_str().to_owned(),
399+
state: state.to_owned(),
370400
};
371401
Some((path, assignment))
372402
} else {

gix-attributes/Cargo.toml

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,27 +14,35 @@ rust-version = "1.85"
1414
[lib]
1515
doctest = true
1616

17+
[[bench]]
18+
name = "lookup"
19+
harness = false
20+
path = "./benches/lookup.rs"
21+
1722
[features]
1823
## Data structures implement `serde::Serialize` and `serde::Deserialize`.
19-
serde = ["dep:serde", "bstr/serde", "gix-glob/serde", "kstring/serde"]
24+
serde = ["dep:serde", "bstr/serde", "gix-glob/serde"]
25+
## Enable thread-safety.
26+
parallel = ["gix-features/parallel"]
2027

2128
[dependencies]
29+
gix-features = { version = "^0.48.1", path = "../gix-features" }
2230
gix-path = { version = "^0.12.2", path = "../gix-path" }
2331
gix-quote = { version = "^0.7.2", path = "../gix-quote" }
2432
gix-glob = { version = "^0.26.1", path = "../gix-glob" }
2533
gix-trace = { version = "^0.1.20", path = "../gix-trace" }
2634

2735
bstr = { version = "1.12.0", default-features = false, features = ["std", "unicode"] }
2836
smallvec = "1.15.1"
29-
kstring = "2.0.0"
3037
unicode-bom = { version = "2.0.3" }
3138
thiserror = "2.0.18"
3239
serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] }
3340

3441
document-features = { version = "0.2.1", optional = true }
3542

3643
[dev-dependencies]
37-
gix-testtools = { path = "../tests/tools" }
44+
criterion = "0.7.0"
45+
gix-testtools = { path = "../tests/tools", default-features = false }
3846
gix-fs = { path = "../gix-fs" }
3947

4048
[package.metadata.docs.rs]

0 commit comments

Comments
 (0)