fix(gix-pack): retain additions from all merge parents
Unified · Split
| @@ -2294,6 +2294,7 @@ dependencies = [ · 2293 unmodified lines | |||
| 2294 | "gix-testtools", | 2294 | "gix-testtools", |
| 2295 | "gix-traverse", | 2295 | "gix-traverse", |
| 2296 | "gix-zlib", | 2296 | "gix-zlib", |
| 2297 | "insta", | ||
| 2297 | "maplit", | 2298 | "maplit", |
| 2298 | "memmap2", | 2299 | "memmap2", |
| 2299 | "parking_lot", | 2300 | "parking_lot", |
| @@ -41,6 +41,8 @@ serde = ["dep:serde", "gix-object/serde", "gix-zlib/serde"] · 40 unmodified lines | |||
| 41 | parallel = ["gix-features/parallel", "dep:crossbeam-deque"] | 41 | parallel = ["gix-features/parallel", "dep:crossbeam-deque"] |
| 42 | ## Make it possible to compile to the `wasm32-unknown-unknown` target. | 42 | ## Make it possible to compile to the `wasm32-unknown-unknown` target. |
| 43 | wasm = ["gix-diff?/wasm"] | 43 | wasm = ["gix-diff?/wasm"] |
| 44 | ## Add test-utilities for tests and benchmarks. | ||
| 45 | testing = [] | ||
| 44 | 46 | ||
| 45 | [dependencies] | 47 | [dependencies] |
| 46 | gix-features = { version = "^0.49.1", path = "../gix-features", features = ["crc32", "progress"] } | 48 | gix-features = { version = "^0.49.1", path = "../gix-features", features = ["crc32", "progress"] } |
| @@ -75,14 +77,16 @@ document-features = { version = "0.2.0", optional = true } · 28 unmodified lines | |||
| 75 | gix-tempfile = { version = "^24.0.0", default-features = false, path = "../gix-tempfile", optional = true } | 77 | gix-tempfile = { version = "^24.0.0", default-features = false, path = "../gix-tempfile", optional = true } |
| 76 | 78 | ||
| 77 | [dev-dependencies] | 79 | [dev-dependencies] |
| 78 | gix-pack = { path = ".", features = ["sha1", "sha256", "pack-cache-lru-dynamic", "pack-cache-lru-static", "object-cache-dynamic", "serde"] } | 80 | gix-pack = { path = ".", features = ["sha1", "sha256", "pack-cache-lru-dynamic", "pack-cache-lru-static", "object-cache-dynamic", "serde", "testing"] } |
| 79 | gix-testtools = { path = "../tests/tools", default-features = false, features = ["sha1", "sha256"] } | 81 | gix-testtools = { path = "../tests/tools", default-features = false, features = ["sha1", "sha256"] } |
| 80 | gix-odb = { path = "../gix-odb", features = ["sha1", "sha256"] } | 82 | gix-odb = { path = "../gix-odb", features = ["sha1", "sha256"] } |
| 81 | gix-object = { path = "../gix-object" } | 83 | gix-object = { path = "../gix-object" } |
| 82 | gix-traverse = { path = "../gix-traverse" } | 84 | gix-traverse = { path = "../gix-traverse" } |
| 85 | |||
| 83 | bstr = { version = "1.12.0", default-features = false, features = ["std"] } | 86 | bstr = { version = "1.12.0", default-features = false, features = ["std"] } |
| 84 | maplit = "1.0.2" | 87 | maplit = "1.0.2" |
| 85 | criterion = "0.8.2" | 88 | criterion = "0.8.2" |
| 89 | insta = "1.46.3" | ||
| 86 | 90 | ||
| 87 | [package.metadata.docs.rs] | 91 | [package.metadata.docs.rs] |
| 88 | all-features = true | 92 | all-features = true |
| @@ -7,68 +7,22 @@ · 6 unmodified lines | |||
| 7 | //! | 7 | //! |
| 8 | //! Note that the discovery phase is as fast as the object database can traverse objects, so nothing to test here | 8 | //! Note that the discovery phase is as fast as the object database can traverse objects, so nothing to test here |
| 9 | //! and we use in-memory speeds for this. | 9 | //! and we use in-memory speeds for this. |
| 10 | use std::{ | 10 | use std::{io, sync::atomic::AtomicBool}; |
| 11 | io, | ||
| 12 | sync::{Arc, atomic::AtomicBool}, | ||
| 13 | }; | ||
| 14 | 11 | ||
| 15 | use criterion::{BatchSize, BenchmarkId, Criterion, Throughput, criterion_group}; | 12 | use criterion::{BatchSize, BenchmarkId, Criterion, Throughput, criterion_group}; |
| 16 | use gix_features::{parallel::InOrderIter, progress}; | 13 | use gix_features::{parallel::InOrderIter, progress}; |
| 17 | use gix_object::Write as _; | 14 | use gix_object::Write as _; |
| 18 | use gix_pack::data::output::{self, bytes::FromEntriesIter, count, entry}; | 15 | use gix_pack::{ |
| 16 | data::output::{self, bytes::FromEntriesIter, count, entry}, | ||
| 17 | testing::Memory, | ||
| 18 | }; | ||
| 19 | 19 | ||
| 20 | /// Object counts to exercise. Kept modest so the benchmark stays runnable while still showing how | 20 | /// Object counts to exercise. Kept modest so the benchmark stays runnable while still showing how |
| 21 | /// the phases scale; raise locally to probe larger, more degenerate repositories. | 21 | /// the phases scale; raise locally to probe larger, more degenerate repositories. |
| 22 | const OBJECT_COUNTS: &[usize] = &[1_000, 10_000, 50_000]; | 22 | const OBJECT_COUNTS: &[usize] = &[1_000, 10_000, 50_000]; |
| 23 | 23 | ||
| 24 | /// Adapt in-memory objects to `gix_pack::Find`. `gix_odb::memory::Proxy` is still used to populate | ||
| 25 | /// the storage, but `count` and `write` need pack-location APIs as well as cheap clones for write | ||
| 26 | /// workers. | ||
| 27 | #[derive(Clone)] | ||
| 28 | struct MemoryOdb { | ||
| 29 | objects: Arc<gix_odb::memory::Storage>, | ||
| 30 | } | ||
| 31 | |||
| 32 | impl gix_pack::Find for MemoryOdb { | ||
| 33 | fn contains(&self, id: &gix_hash::oid) -> bool { | ||
| 34 | self.objects.contains_key(id) | ||
| 35 | } | ||
| 36 | |||
| 37 | fn try_find_cached<'a>( | ||
| 38 | &self, | ||
| 39 | id: &gix_hash::oid, | ||
| 40 | buffer: &'a mut Vec<u8>, | ||
| 41 | _pack_cache: &mut dyn gix_pack::cache::DecodeEntry, | ||
| 42 | ) -> Result<Option<(gix_object::Data<'a>, Option<gix_pack::data::entry::Location>)>, gix_object::find::Error> { | ||
| 43 | Ok(self.objects.get(id).map(|(kind, data)| { | ||
| 44 | buffer.clear(); | ||
| 45 | buffer.extend_from_slice(data); | ||
| 46 | ( | ||
| 47 | gix_object::Data { | ||
| 48 | kind: *kind, | ||
| 49 | object_hash: id.kind(), | ||
| 50 | data: buffer, | ||
| 51 | }, | ||
| 52 | None, | ||
| 53 | ) | ||
| 54 | })) | ||
| 55 | } | ||
| 56 | |||
| 57 | fn location_by_oid(&self, _id: &gix_hash::oid, _buf: &mut Vec<u8>) -> Option<gix_pack::data::entry::Location> { | ||
| 58 | None | ||
| 59 | } | ||
| 60 | |||
| 61 | fn pack_offsets_and_oid(&self, _pack_id: u32) -> Option<Vec<(gix_pack::data::Offset, gix_hash::ObjectId)>> { | ||
| 62 | None | ||
| 63 | } | ||
| 64 | |||
| 65 | fn entry_by_location(&self, _location: &gix_pack::data::entry::Location) -> Option<gix_pack::find::Entry> { | ||
| 66 | None | ||
| 67 | } | ||
| 68 | } | ||
| 69 | |||
| 70 | /// Create a fresh in-memory object database populated with `count` unique blobs. | 24 | /// Create a fresh in-memory object database populated with `count` unique blobs. |
| 71 | fn memory_odb(count: usize) -> (MemoryOdb, Vec<gix_hash::ObjectId>) { | 25 | fn memory_odb(count: usize) -> (Memory, Vec<gix_hash::ObjectId>) { |
| 72 | let mut odb = gix_odb::memory::Proxy::new(gix_odb::sink(gix_hash::Kind::Sha1), gix_hash::Kind::Sha1); | 26 | let mut odb = gix_odb::memory::Proxy::new(gix_odb::sink(gix_hash::Kind::Sha1), gix_hash::Kind::Sha1); |
| 73 | let mut ids = Vec::with_capacity(count); | 27 | let mut ids = Vec::with_capacity(count); |
| 74 | for i in 0..count { | 28 | for i in 0..count { |
| @@ -78,19 +32,14 @@ fn memory_odb(count: usize) -> (MemoryOdb, Vec<gix_hash::ObjectId>) { · 3 unmodified lines | |||
| 78 | .expect("can write an in-memory object"); | 32 | .expect("can write an in-memory object"); |
| 79 | ids.push(id); | 33 | ids.push(id); |
| 80 | } | 34 | } |
| 81 | let objects = odb | 35 | let mut objects = odb |
| 82 | .take_object_memory() | 36 | .take_object_memory() |
| 83 | .expect("in-memory object storage is still enabled"); | 37 | .expect("in-memory object storage is still enabled"); |
| 84 | ( | 38 | (Memory::new(objects.drain()), ids) |
| 85 | MemoryOdb { | ||
| 86 | objects: Arc::new(objects), | ||
| 87 | }, | ||
| 88 | ids, | ||
| 89 | ) | ||
| 90 | } | 39 | } |
| 91 | 40 | ||
| 92 | /// Phase 1: collect the counts for `ids` without object expansion - exactly the objects given. | 41 | /// Phase 1: collect the counts for `ids` without object expansion - exactly the objects given. |
| 93 | fn count_objects_unthreaded(odb: &MemoryOdb, ids: &[gix_hash::ObjectId]) -> Vec<output::Count> { | 42 | fn count_objects_unthreaded(odb: &Memory, ids: &[gix_hash::ObjectId]) -> Vec<output::Count> { |
| 94 | let mut input = ids.iter().copied().map(Ok); | 43 | let mut input = ids.iter().copied().map(Ok); |
| 95 | let (counts, _outcome) = count::objects_unthreaded( | 44 | let (counts, _outcome) = count::objects_unthreaded( |
| 96 | odb, | 45 | odb, |
| @@ -120,7 +69,7 @@ impl io::Write for CountingSink { · 23 unmodified lines | |||
| 120 | } | 69 | } |
| 121 | 70 | ||
| 122 | /// Phase 2: resolve, sort and encode `counts` into a pack stream, returning the pack size in bytes. | 71 | /// Phase 2: resolve, sort and encode `counts` into a pack stream, returning the pack size in bytes. |
| 123 | fn write_pack(odb: &MemoryOdb, counts: Vec<output::Count>) -> u64 { | 72 | fn write_pack(odb: &Memory, counts: Vec<output::Count>) -> u64 { |
| 124 | let num_objects = counts.len() as u32; | 73 | let num_objects = counts.len() as u32; |
| 125 | let entries = InOrderIter::from(entry::iter_from_counts( | 74 | let entries = InOrderIter::from(entry::iter_from_counts( |
| 126 | counts, | 75 | counts, |
| @@ -220,6 +220,7 @@ mod expand { · 219 unmodified lines | |||
| 220 | out = objects.dissolve(stats); | 220 | out = objects.dissolve(stats); |
| 221 | &traverse_delegate.non_trees | 221 | &traverse_delegate.non_trees |
| 222 | } else { | 222 | } else { |
| 223 | changes_delegate.clear(); | ||
| 223 | for commit_id in &parent_commit_ids { | 224 | for commit_id in &parent_commit_ids { |
| 224 | let parent_tree_id = { | 225 | let parent_tree_id = { |
| 225 | let (parent_commit_obj, location) = db.find(commit_id, buf2)?; | 226 | let (parent_commit_obj, location) = db.find(commit_id, buf2)?; |
| @@ -251,7 +252,6 @@ mod expand { · 25 unmodified lines | |||
| 251 | ) | 252 | ) |
| 252 | }; | 253 | }; |
| 253 | 254 | ||
| 254 | changes_delegate.clear(); | ||
| 255 | let objects = CountingObjects::new(db); | 255 | let objects = CountingObjects::new(db); |
| 256 | gix_diff::tree( | 256 | gix_diff::tree( |
| 257 | parent_tree, | 257 | parent_tree, |
| @@ -57,6 +57,10 @@ pub mod multi_index; · 56 unmodified lines | |||
| 57 | /// | 57 | /// |
| 58 | pub mod verify; | 58 | pub mod verify; |
| 59 | 59 | ||
| 60 | /// | ||
| 61 | #[cfg(feature = "testing")] | ||
| 62 | pub mod testing; | ||
| 63 | |||
| 60 | mod mmap { | 64 | mod mmap { |
| 61 | use std::path::Path; | 65 | use std::path::Path; |
| 62 | 66 | ||
| @@ -0,0 +1,51 @@ | |||
| 1 | use crate::find::Entry; | ||
| 2 | |||
| 3 | /// An in-memory object database without pack locations. | ||
| 4 | #[derive(Clone)] | ||
| 5 | pub struct Memory { | ||
| 6 | objects: std::sync::Arc<gix_hashtable::HashMap<gix_hash::ObjectId, (gix_object::Kind, Vec<u8>)>>, | ||
| 7 | } | ||
| 8 | |||
| 9 | impl Memory { | ||
| 10 | /// Create a database containing all `objects`. | ||
| 11 | /// | ||
| 12 | /// Each item is `(id, (kind, data))`, where `data` is the uncompressed object body without a | ||
| 13 | /// loose-object header. IDs are trusted rather than recomputed. If an ID occurs more than once, | ||
| 14 | /// the last item wins. | ||
| 15 | pub fn new(objects: impl IntoIterator<Item = (gix_hash::ObjectId, (gix_object::Kind, Vec<u8>))>) -> Self { | ||
| 16 | Self { | ||
| 17 | objects: std::sync::Arc::new(objects.into_iter().collect()), | ||
| 18 | } | ||
| 19 | } | ||
| 20 | } | ||
| 21 | |||
| 22 | impl crate::Find for Memory { | ||
| 23 | fn contains(&self, id: &gix_hash::oid) -> bool { | ||
| 24 | self.objects.contains_key(id) | ||
| 25 | } | ||
| 26 | |||
| 27 | fn try_find_cached<'a>( | ||
| 28 | &self, | ||
| 29 | id: &gix_hash::oid, | ||
| 30 | buffer: &'a mut Vec<u8>, | ||
| 31 | _pack_cache: &mut dyn crate::cache::DecodeEntry, | ||
| 32 | ) -> Result<Option<(gix_object::Data<'a>, Option<crate::data::entry::Location>)>, gix_object::find::Error> { | ||
| 33 | Ok(self.objects.get(id).map(|(kind, data)| { | ||
| 34 | buffer.clear(); | ||
| 35 | buffer.extend_from_slice(data); | ||
| 36 | (gix_object::Data::new(buffer, *kind, id.kind()), None) | ||
| 37 | })) | ||
| 38 | } | ||
| 39 | |||
| 40 | fn location_by_oid(&self, _id: &gix_hash::oid, _buf: &mut Vec<u8>) -> Option<crate::data::entry::Location> { | ||
| 41 | None | ||
| 42 | } | ||
| 43 | |||
| 44 | fn pack_offsets_and_oid(&self, _pack_id: u32) -> Option<Vec<(crate::data::Offset, gix_hash::ObjectId)>> { | ||
| 45 | None | ||
| 46 | } | ||
| 47 | |||
| 48 | fn entry_by_location(&self, _location: &crate::data::entry::Location) -> Option<Entry> { | ||
| 49 | None | ||
| 50 | } | ||
| 51 | } | ||
| @@ -367,6 +367,81 @@ fn traversals() -> crate::Result { · 366 unmodified lines | |||
| 367 | Ok(()) | 367 | Ok(()) |
| 368 | } | 368 | } |
| 369 | 369 | ||
| 370 | #[test] | ||
| 371 | fn tree_additions_from_each_merge_parent_are_kept() -> crate::Result { | ||
| 372 | use gix_object::Write; | ||
| 373 | |||
| 374 | let object_hash = object_hash(); | ||
| 375 | let mut db = gix_odb::memory::Proxy::new(gix_odb::sink(object_hash), object_hash); | ||
| 376 | let added_blob_id = db.write_buf(gix_object::Kind::Blob, b"added")?; | ||
| 377 | let base_tree_id = db.write(&gix_object::Tree::empty())?; | ||
| 378 | let merged_tree_id = db.write(&gix_object::Tree { | ||
| 379 | entries: vec![gix_object::tree::Entry { | ||
| 380 | mode: gix_object::tree::EntryKind::Blob.into(), | ||
| 381 | oid: added_blob_id, | ||
| 382 | filename: "added".into(), | ||
| 383 | }], | ||
| 384 | })?; | ||
| 385 | let write_commit = |tree, parents: &[gix_hash::ObjectId]| { | ||
| 386 | db.write(&gix_object::Commit { | ||
| 387 | tree, | ||
| 388 | parents: parents.iter().copied().collect(), | ||
| 389 | author: Default::default(), | ||
| 390 | committer: Default::default(), | ||
| 391 | encoding: None, | ||
| 392 | message: "commit".into(), | ||
| 393 | extra_headers: Vec::new(), | ||
| 394 | }) | ||
| 395 | }; | ||
| 396 | let base_commit_id = write_commit(base_tree_id, &[])?; | ||
| 397 | let feature_commit_id = write_commit(merged_tree_id, &[base_commit_id])?; | ||
| 398 | let merge_commit_id = write_commit(merged_tree_id, &[base_commit_id, feature_commit_id])?; | ||
| 399 | let mut objects = db | ||
| 400 | .take_object_memory() | ||
| 401 | .expect("in-memory object storage is still enabled"); | ||
| 402 | let db = gix_pack::testing::Memory::new(objects.drain()); | ||
| 403 | let mut input = std::iter::once(Ok::<_, Box<dyn std::error::Error + Send + Sync>>(merge_commit_id)); | ||
| 404 | |||
| 405 | let (counts, stats) = output::count::objects_unthreaded( | ||
| 406 | &db, | ||
| 407 | &mut input, | ||
| 408 | &progress::Discard, | ||
| 409 | &AtomicBool::new(false), | ||
| 410 | count::objects::ObjectExpansion::TreeAdditionsComparedToAncestor, | ||
| 411 | )?; | ||
| 412 | |||
| 413 | let expected_ids = std::collections::BTreeSet::from([ | ||
| 414 | added_blob_id, | ||
| 415 | base_tree_id, | ||
| 416 | merged_tree_id, | ||
| 417 | base_commit_id, | ||
| 418 | feature_commit_id, | ||
| 419 | merge_commit_id, | ||
| 420 | ]); | ||
| 421 | assert_eq!( | ||
| 422 | counts.len(), | ||
| 423 | expected_ids.len(), | ||
| 424 | "each expected object must be counted exactly once" | ||
| 425 | ); | ||
| 426 | assert_eq!( | ||
| 427 | counts | ||
| 428 | .iter() | ||
| 429 | .map(|count| count.id) | ||
| 430 | .collect::<std::collections::BTreeSet<_>>(), | ||
| 431 | expected_ids, | ||
| 432 | "additions found against every merge parent must remain in the pack" | ||
| 433 | ); | ||
| 434 | insta::assert_debug_snapshot!(stats, @" | ||
| 435 | Outcome { | ||
| 436 | input_objects: 1, | ||
| 437 | expanded_objects: 5, | ||
| 438 | decoded_objects: 5, | ||
| 439 | total_objects: 6, | ||
| 440 | } | ||
| 441 | "); | ||
| 442 | Ok(()) | ||
| 443 | } | ||
| 444 | |||
| 370 | /// Reproduces https://github.com/GitoxideLabs/gitoxide/issues/2024: with the default backend's | 445 | /// Reproduces https://github.com/GitoxideLabs/gitoxide/issues/2024: with the default backend's |
| 371 | /// level 1 being much weaker than it used to be, entries have to be compressed with the | 446 | /// level 1 being much weaker than it used to be, entries have to be compressed with the |
| 372 | /// configured level, defaulting to what `git` uses. | 447 | /// configured level, defaulting to what `git` uses. |