[PATCH v3 0/8] reftable: fixes and optimizations (pt.2)

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