The loose header manipulation currently lives in git-pack but it depends on nothing *of* git-pack. Move it to git-object (`::encode` and `::decode`), as well as update the way it's generated: a header should be no more than 28 bytes: 6 bytes for the kind, 20 for the size (log10(2**64) = 19.3), 1 for the space and 1 for the NUL, so return a `SmallVec<[u8, 28]>` instead of writing it out directly (this should slightly reduce the amount of IO when writing out the header to an unbuffered stream), this also avoids the need for `header_buf` in the traversal state. In order to generate the header without having to write out the entire content, add `WriteTo::size()`, the size is (relatively) easy to compute for all kinds. Ultimately this should avoid having to buffer or move around the object data before generating the header (though that's a bit TBD, I don't remember making those changes in git-pack). This also requires adding size computations to `git_actor::Signature` and `git_actor::Time`. For the latter the result should be reasonably efficient[^bithack]. If the time part gets moved to 64b, this should probably be updated to use a lookup table[^lookup] or even better `u64::log10` as hopefully it'll be stable by then[^70887]. Also add direct shortcuts to `WriteTo` (to generate a loose header) and `ObjectRef` (to directly parse a loose object). Others: * Lift conversion and header encoding at the start of `Sink::write_stream` as they don't seem to depend on the hash kind. * Alter `loose::Store` to inlin `write_header` but extract the creation of the output stream, this reveals that the dispatch on hash kind wasn't useful anymore as only the creation of the stream needs it (aside from `finalize_object`). One concern I found when making this change is things are kinda broken wrt 32/64b: the existing code is a bit mixed between usize and u64, but tends to store data in buffers which work by usize anyway. But of course using usize means broken / corrupted files > 4GB on 32b platforms, which is not great either. Then again git itself has the same issue except even worse: it uses `unsigned long` internally, which is not only 32b on 32b platforms (ILP32) but also on 64 bits windows (LLP64)... Final note: the `cargo fmt` for commits and tags is *really bad* as it puts one sub-expression per line, a better alternative might be to provide size-only and `[inline]` versions of the encoding helpers? [^bithack]: it's considered a good solution when the input is uniformly distributed (http://graphics.stanford.edu/~seander/bithacks.html#IntegerLog10Obvious), and for us the input is biased toward efficiency: timestamp 1e9 is September 2001, and timestamp 1e8 is March 1973, so the vast majority of legit commits will take the first branch, and only joke commits will fail the second really. [^lookup]: https://commaok.xyz/post/lookup_tables/ however these solve the problem for u32 [^70887]: https://github.com/rust-lang/rust/issues/70887 and efficiency improvements could be contributed to the stdlib directly.
Showing the changes to git-object/src/commit/write.rs (1 of 24 files). Show all
Unified · Split
| @@ -1,3 +1,4 @@ | |||
| 1 | use bstr::ByteSlice; | ||
| 1 | use std::io; | 2 | use std::io; |
| 2 | 3 | ||
| 3 | use crate::{encode, encode::NL, Commit, CommitRef, Kind}; | 4 | use crate::{encode, encode::NL, Commit, CommitRef, Kind}; |
| @@ -21,6 +22,38 @@ impl crate::WriteTo for Commit { · 17 unmodified lines | |||
| 21 | out.write_all(&self.message) | 22 | out.write_all(&self.message) |
| 22 | } | 23 | } |
| 23 | 24 | ||
| 25 | fn size(&self) -> usize { | ||
| 26 | let hashsize = self.tree.kind().len_in_hex(); | ||
| 27 | b"tree".len() | ||
| 28 | + 1 | ||
| 29 | + hashsize | ||
| 30 | + 1 | ||
| 31 | + self.parents.iter().count() * (b"parent".len() + 1 + hashsize + 1) | ||
| 32 | + b"author".len() | ||
| 33 | + 1 | ||
| 34 | + self.author.size() | ||
| 35 | + 1 | ||
| 36 | + b"committer".len() | ||
| 37 | + 1 | ||
| 38 | + self.committer.size() | ||
| 39 | + 1 | ||
| 40 | + self | ||
| 41 | .encoding | ||
| 42 | .as_ref() | ||
| 43 | .map(|e| b"encoding".len() + 1 + e.len() + 1) | ||
| 44 | .unwrap_or(0) | ||
| 45 | + self | ||
| 46 | .extra_headers | ||
| 47 | .iter() | ||
| 48 | .map(|(name, value)| { | ||
| 49 | // each header *value* is preceded by a space and followed by a newline | ||
| 50 | name.len() + value.split_str("\n").map(|s| s.len() + 2).sum::<usize>() | ||
| 51 | }) | ||
| 52 | .sum::<usize>() | ||
| 53 | + 1 | ||
| 54 | + self.message.len() | ||
| 55 | } | ||
| 56 | |||
| 24 | fn kind(&self) -> Kind { | 57 | fn kind(&self) -> Kind { |
| 25 | Kind::Commit | 58 | Kind::Commit |
| 26 | } | 59 | } |
| @@ -45,6 +78,38 @@ impl<'a> crate::WriteTo for CommitRef<'a> { · 18 unmodified lines | |||
| 45 | out.write_all(self.message) | 78 | out.write_all(self.message) |
| 46 | } | 79 | } |
| 47 | 80 | ||
| 81 | fn size(&self) -> usize { | ||
| 82 | let hashsize = self.tree().kind().len_in_hex(); | ||
| 83 | b"tree".len() | ||
| 84 | + 1 | ||
| 85 | + hashsize | ||
| 86 | + 1 | ||
| 87 | + self.parents.iter().count() * (b"parent".len() + 1 + hashsize + 1) | ||
| 88 | + b"author".len() | ||
| 89 | + 1 | ||
| 90 | + self.author.size() | ||
| 91 | + 1 | ||
| 92 | + b"committer".len() | ||
| 93 | + 1 | ||
| 94 | + self.committer.size() | ||
| 95 | + 1 | ||
| 96 | + self | ||
| 97 | .encoding | ||
| 98 | .as_ref() | ||
| 99 | .map(|e| b"encoding".len() + 1 + e.len() + 1) | ||
| 100 | .unwrap_or(0) | ||
| 101 | + self | ||
| 102 | .extra_headers | ||
| 103 | .iter() | ||
| 104 | .map(|(name, value)| { | ||
| 105 | // each header *value* is preceded by a space and followed by a newline | ||
| 106 | name.len() + value.split_str("\n").map(|s| s.len() + 2).sum::<usize>() | ||
| 107 | }) | ||
| 108 | .sum::<usize>() | ||
| 109 | + 1 | ||
| 110 | + self.message.len() | ||
| 111 | } | ||
| 112 | |||
| 48 | fn kind(&self) -> Kind { | 113 | fn kind(&self) -> Kind { |
| 49 | Kind::Commit | 114 | Kind::Commit |
| 50 | } | 115 | } |