-
-
Notifications
You must be signed in to change notification settings - Fork 321
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
pack-generation MVP #67
Comments
Related to #53 |
In pursuit of better control pack generation and also pave the way for improved Now it's possible to step through parallel computations. However, the respective implementation has to expose Depending on where this is exposed, My intuition is to stop bubbling this up beyond |
It's a CPU-intensive operation; my first instinct would be to run it normally and use unblock or similar to run it on a blocking thread. Trying to structure the computation so that it happens incrementally seems incredibly painful. And in particular, trying to adapt an operation that happens in a thread to happen incrementally seems like it's incurring all the pain of async without any language support for async. I would suggest building the initial MVP in a synchronous fashion, on the theory that it can still be run in a background thread and controlled via an async mechanism. I definitely don't think it's OK to use a scoped thread and hide the unsafe, if the unsafe isn't truly encapsulated to the point that you can't do anything unsound with the interface. |
One other thought on that front: compared to the cost of generating a pack, one or two allocations to set up things like channels or Arc will not be an issue. |
Thanks for sharing. The main motivator for using scoped threads is to allow standard stack based operation without any wrapping - it's not at all about allocations, merely about usability and the least surprising behaviour. Truth to be told, I cannot currently imagine how traversal will play into static threads when arcs are involved especially along with traits representing an Object (an attempt to allow things like traversing directory trees). What I take away is the following
I hope to overcome my 'writers block' and just write the missing bits to be able to see through the whole operation and play with the parts more until I find a version of the API that feels right. |
The
This capability probably doesn't have to be removed just yet as machinery itself is exactly the same as already used in However, it's possible to eventually provide a non-static variant of pack generation too which works similar to pack verification (it uses the non-static version of the machinery) by factoring out the parts that are similar. Another argument for trying hard to make pack generation play well in an async context certainly is that it commonly happens as part of network interactions like uploading a pack. Right now much of it is a little hypothetical as actual code to prove it works nicely doesn't actually exist, but I am confident it will work as envisioned. Finally, since both machines, static and non-static are the same at core it should always be possible to return to the non-static one at very low cost should everything else fail. |
On another note: I am also thinking backpressure and and back-communication. Backpressure is already present as threads will block once the results channel is full. Back-communication should also be possible if the handed-in closures get access to another synchronized channel of sorts to tell them when to deliver the pack entries they have been working on. Such an algorithm would continuously work (probably until it can't meaningfully improve the deltas) until it is told to deliver what's there right now before continuing. Such a message could then be delivered in the moment somebody actually calls Even though the MVP will not do back-communication I don't see why it shouldn't be possible to implement it. What's neat is that no matter how the machinery operates, in the moment the iterator is dropped it will (potentially with some delay), stop working automatically. |
The first break-through: pack files (base object only) can now be written from object ids. |
…and it's nice to see that overall, it's very flexible even though it may require acrobatics to get it in and out of Easy mode.
…we could also double-check the produced crc32 values, but assigning consecutive numbers seems like it's doing the job.
…ing… (#67) …which is what happens when counting objects for fetches where only changed objects should be sent.
…to avoid dealing with missing objects. It's still a good idea to handle these gracefully though, git itself seems to ignore them.
…for about 10% of performance, speeding up these lookups just a little bit.
…by implementing the few bits that are needed ourselves. This might open up future optimizations, if they matter. Controlling the nom parsers on a token by token basis via iterators probably already saves a lot of time, and these are used everywhere.
Intead, share it by reference, it's sync after all. This issue was introduced when switching to a `Send + Clone` model, instead of `Send + Sync`, to allow thread-local caches in database handles of all kinds.
…nting (#67) The efficiency of multi-threaded counting is low per core, and despite some speedups might be desirable, one might not want to commit all cores to this amount of waste.
About opportunities for performance improvements@pascalkuthe I have created quick profile from running My takeaways are as follows:
This is just a quick summary, and right now I am missing a dataset to compare git with While at it, profiling |
gen-pack like plumbing command
Generate a pack using some selection of commits or possibly objects. Drives different kinds of iteration as well as ways of building a pack.
Progress
gixp pack-create
Easy*
hash_hasher
for single-threaded counting to see how it performs compared to the standard hash-set with hasher override.prodash
will cause slowdown.ignore_replacements
. It's correct to ignore replacements during reachability traversal, i.e. packpack-index-from-data
.Command-lines
Out of scope
reuse existing deltas - doing so would allow to save a lot of time and avoids the need for implementing actual delta compression. One would only have to get the order of entries right to assure consistency.Without that it's not really usable.gixp
subcommand to make the functionality available for stress testing and performance testing*Machinery to produce object deltas.
User Stories
Interesting
Archive
Progress
stream pack entries (base objects only) from an iterator of input objects
write pack entries to a V2 pack
clone-like - the other side has no objects
fetch-like - the other side has some objects
Handle tag objects - their pointee must be added to the set as well.
Make it so that the total amount of objects to be written is known in advance
Statistics about re-used objects and repacked ones, maybe more. This is critical to eventually avoid repacking most.
--statistics
restore chunk-ordering to allow multi-threaded generators to yield the same pack all the time (chunks may be out of order)
figure out why the pack hash in parallel mode is still not reproducible
write object entries directly, without forcing to copy their compressed data into anoutput::Entry
re-use delta objects as well and write their offsets correctly.
gixp pack-create
git rev-list
) (AsIs traversal is default here)Count objects performance improvement #167
Improving counting performance #170
Performance Opportunities
it would save a lot of time if we would know where the next entry begins.https://github.com/Byron/gitoxide/blob/34b6a2e94949b24bf0bbaeb169b4baa0fa45c965/git-odb/src/linked/find.rs#L52
Notes
packs in general
git_odb::compound::Db::locate(…)
#66 .Pack generation
These threads regularly pause their work to allow the stealing to happen safely which might be why that doesn't scale?(actually that one does seem to scale at least with the amount of core I have, it's pack resolution that doesn't scale)The text was updated successfully, but these errors were encountered: