Avoid duplicate hash computation during object creation#2549
Conversation
Sebastian Thiel (Byron)
force-pushed
the
no-dupl-compute
branch
from
358e6cd to
3f11f85
Compare
Sebastian Thiel (Byron)
force-pushed
the
no-dupl-compute
branch
from
3f11f85 to
f86f8d7
Compare
Jon Ross-Perkins (jonmeow)
left a comment
Jon Ross-Perkins (jonmeow)
left a comment
There was a problem hiding this comment.
Thanks! This looks good to me, just a couple questions about the Write trait changes (TBH I'm really just comparing with the non-known_id variants)
|
Jon Ross-Perkins (@jonmeow) Thanks so much for taking a look! I haven't looked at the code at all yet, but when I do I will particularly look at the places you are highlighting. |
Add `write_buf_with_known_id()` and `write_stream_with_known_id()` to the Write trait without default bodies. That way it's possible to precompute the hash and avoid recomputing it here. Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
This means the `Repository::write_object()` won't recalculate the hash. Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Sebastian Thiel (Byron)
force-pushed
the
no-dupl-compute
branch
from
f86f8d7 to
2bd9dfe
Compare
There was a problem hiding this comment.
Pull request overview
Introduces write_buf_with_known_id and write_stream_with_known_id on gix_object::Write so callers that already computed an object's hash (e.g. for the existence pre-check in Repository::write_*) can skip the duplicate hash computation performed by the ODB. All ODB layers (sink, loose store, dynamic store, cache, memory proxy) and the &T/Arc/Rc forwarders are updated, and the Repository write paths are switched to the new entry points.
Changes:
- Adds two new methods to the
gix_object::Writetrait and forwards them through all wrapper impls. - Implements known-ID write paths in sink, loose store (refactored into
compressed_tempfile/finalize_object_at), dynamic store, cache, and memory proxy. - Rewires
Repository::write_object,write_blob,write_blob_stream, andimpl Write for Repositoryto use the known ID, with tests covering loose store and memory proxy.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| gix-object/src/traits/mod.rs | Adds the two new (required) trait methods. |
| gix-object/src/traits/_impls.rs | Forwards new methods through &T, Arc<T>, Rc<T>. |
| gix-odb/src/sink.rs | Implements known-ID variants by mirroring the existing compress loop without hashing. |
| gix-odb/src/store_impls/loose/write.rs | Splits hash wrapping/finalization so known-ID writes can bypass hashing. |
| gix-odb/src/store_impls/dynamic/write.rs | Forwards known-ID writes to the first loose DB, initializing if needed. |
| gix-odb/src/cache.rs | Delegates known-ID writes to inner. |
| gix-odb/src/memory.rs | Inserts directly into in-memory map using the given ID, else forwards. |
| gix/src/repository/object.rs | Switches three write sites to write_buf_with_known_id. |
| gix/src/repository/impls.rs | Implements known-ID variants on Repository, including stream-size validation. |
| gix-odb/tests/odb/store/loose.rs | Round-trips known-ID buf/stream writes. |
| gix-odb/tests/odb/memory.rs | Verifies known-ID is trusted (with/without in-memory map). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes #2516
Tasks