Hi,
this is the third version of my patch series that contains additional
fixes and optimizations for the reftable library.
The only changes in this iteration are improvements to the commit
messages to hopefully make them easier to understand. Thanks Junio for
your suggestions!
Patrick
Patrick Steinhardt (8):
reftable/stack: do not overwrite errors when compacting
reftable/stack: do not auto-compact twice in `reftable_stack_add()`
reftable/writer: fix index corruption when writing multiple indices
reftable/record: constify some parts of the interface
reftable/record: store "val1" hashes as static arrays
reftable/record: store "val2" hashes as static arrays
reftable/merged: really reuse buffers to compute record keys
reftable/merged: transfer ownership of records when iterating
reftable/block_test.c | 4 +-
reftable/merged.c | 8 +--
reftable/merged_test.c | 16 +++---
reftable/readwrite_test.c | 102 +++++++++++++++++++++++++++++++------
reftable/record.c | 17 ++-----
reftable/record_test.c | 5 --
reftable/reftable-record.h | 11 ++--
reftable/stack.c | 23 +++------
reftable/stack_test.c | 2 -
reftable/writer.c | 4 +-
10 files changed, 117 insertions(+), 75 deletions(-)
Range-diff against v2:
1: 22a57020c6 = 1: 1dc8ddf04a reftable/stack: do not overwrite errors when compacting
2: a08f7efc99 = 2: eccc34a4b6 reftable/stack: do not auto-compact twice in `reftable_stack_add()`
3: c00e08d97f = 3: 15e12b8f29 reftable/writer: fix index corruption when writing multiple indices
4: 3416268087 = 4: 017e8943c7 reftable/record: constify some parts of the interface
5: 46ca3a37f8 ! 5: f1d2190489 reftable/record: store "val1" hashes as static arrays
@@ Metadata
## Commit message ##
reftable/record: store "val1" hashes as static arrays
- When reading ref records of type "val1" we store its object ID in an
+ When reading ref records of type "val1", we store its object ID in an
allocated array. This results in an additional allocation for every
single ref record we read, which is rather inefficient especially when
iterating over refs.
- Refactor the code to instead use a static array of `GIT_MAX_RAWSZ`
+ Refactor the code to instead use an embedded array of `GIT_MAX_RAWSZ`
bytes. While this means that `struct ref_record` is bigger now, we
typically do not store all refs in an array anyway and instead only
handle a limited number of records at the same point in time.
6: c8a36917b1 = 6: 6ec02d6775 reftable/record: store "val2" hashes as static arrays
7: 6313f8affd ! 7: 845dec2390 reftable/merged: really reuse buffers to compute record keys
@@ Commit message
reftable/merged: really reuse buffers to compute record keys
In 829231dc20 (reftable/merged: reuse buffer to compute record keys,
- 2023-12-11), we have refactored the merged iterator to reuse a set of
- buffers that it would otherwise have to reallocate on every single
- iteration. Unfortunately, there was a brown-paper-bag-style bug here as
- we continue to release these buffers after the iteration, and thus we
- have essentially gained nothing.
+ 2023-12-11), we have refactored the merged iterator to reuse a pair of
+ long-living strbufs by relying on the fact that `reftable_record_key()`
+ tries to reuse already allocated strbufs by calling `strbuf_reset()`,
+ which should give us significantly fewer reallocations compared to the
+ old code that used on-stack strbufs that are allocated for each and
+ every iteration. Unfortunately, we called `strbuf_release()` on these
+ long-living strbufs that we meant to reuse on each iteration, defeating
+ the optimization.
Fix this performance issue by not releasing those buffers on iteration
anymore, where we instead rely on `merged_iter_close()` to release the
8: 25a3919e58 ! 8: eea0e161ad reftable/merged: transfer ownership of records when iterating
@@ Metadata
## Commit message ##
reftable/merged: transfer ownership of records when iterating
- When iterating over recods with the merged iterator we put the records
+ When iterating over records with the merged iterator we put the records
into a priority queue before yielding them to the caller. This means
that we need to allocate the contents of these records before we can
pass them over to the caller.
base-commit: a26002b62827b89a19b1084bd75d9371d565d03c
--
2.43.GIT
Attachment:
signature.asc
Description: PGP signature
