[PATCH v2 00/11] reftable: optimize write performance

Hi,

this is the second version of my patch series that aims to optimize write performance in the reftable backend. I've made the following changes compared to v2:

  - Added a new patch to drop "reftable/refname.{c,h}" and related
    completely.

  - Reworded the commit message for the committer ident refactorings.

  - Added a new patch that unifies the code paths that release reftable
    writer resources.

  - Fixed a stale comment claiming that we retry deflating, which we
    don't.

Thanks!

Patrick

Patrick Steinhardt (11):
  refs/reftable: fix D/F conflict error message on ref copy
  refs/reftable: perform explicit D/F check when writing symrefs
  refs/reftable: skip duplicate name checks
  reftable: remove name checks
  refs/reftable: don't recompute committer ident
  reftable/writer: refactorings for `writer_add_record()`
  reftable/writer: refactorings for `writer_flush_nonempty_block()`
  reftable/writer: unify releasing memory
  reftable/writer: reset `last_key` instead of releasing it
  reftable/block: reuse zstream when writing log blocks
  reftable/block: reuse compressed array

 Makefile                   |   2 -
 refs/reftable-backend.c    |  75 +++++++++----
 reftable/block.c           |  80 ++++++++------
 reftable/block.h           |   4 +
 reftable/error.c           |   2 -
 reftable/refname.c         | 209 -------------------------------------
 reftable/refname.h         |  29 -----
 reftable/refname_test.c    | 101 ------------------
 reftable/reftable-error.h  |   3 -
 reftable/reftable-tests.h  |   1 -
 reftable/reftable-writer.h |   4 -
 reftable/stack.c           |  67 +-----------
 reftable/stack_test.c      |  39 -------
 reftable/writer.c          | 137 +++++++++++++++---------
 t/helper/test-reftable.c   |   1 -
 t/t0610-reftable-basics.sh |  35 ++++++-
 16 files changed, 230 insertions(+), 559 deletions(-)
 delete mode 100644 reftable/refname.c
 delete mode 100644 reftable/refname.h
 delete mode 100644 reftable/refname_test.c

Range-diff against v1:
 1:  14b4dacd73 =  1:  926e802395 refs/reftable: fix D/F conflict error message on ref copy
 2:  55db366e61 =  2:  6190171906 refs/reftable: perform explicit D/F check when writing symrefs
 3:  ad8210ec65 =  3:  80008cc5e7 refs/reftable: skip duplicate name checks
 -:  ---------- >  4:  3497a570b4 reftable: remove name checks
 4:  a9a6795c02 !  5:  f892a3007b refs/reftable: don't recompute committer ident
    @@ Commit message
         refs/reftable: don't recompute committer ident
     
         In order to write reflog entries we need to compute the committer's
    -    identity as it becomes encoded in the log record itself. In the reftable
    -    backend, computing the identity is repeated for every single reflog
    -    entry which we are about to write in a transaction. Needless to say,
    -    this can be quite a waste of effort when writing many refs with reflog
    -    entries in a single transaction.
    +    identity as it gets encoded in the log record itself. The reftable
    +    backend does this via `git_committer_info()` and `split_ident_line()` in
    +    `fill_reftable_log_record()`, which use the Git config as well as
    +    environment variables to figure out the identity.
    +
    +    While most callers would only call `fill_reftable_log_record()` once or
    +    twice, `write_transaction_table()` will call it as many times as there
    +    are queued ref updates. This can be quite a waste of effort when writing
    +    many refs with reflog entries in a single transaction.
     
         Refactor the code to pre-compute the committer information. This results
         in a small speedup when writing 100000 refs in a single transaction:
 5:  8e9d69e9e6 =  6:  4877ab3921 reftable/writer: refactorings for `writer_add_record()`
 6:  1f903afdda =  7:  8f1c5b4169 reftable/writer: refactorings for `writer_flush_nonempty_block()`
 -:  ---------- >  8:  41db7414e1 reftable/writer: unify releasing memory
 9:  6950ae4ea7 !  9:  e5c7dbe417 reftable/writer: reset `last_key` instead of releasing it
    @@ reftable/writer.c: static void writer_reinit_block_writer(struct reftable_writer
      	block_writer_init(&w->block_writer_data, typ, w->block,
      			  w->opts.block_size, block_start,
      			  hash_size(w->opts.hash_id));
    -@@ reftable/writer.c: void reftable_writer_free(struct reftable_writer *w)
    - 		block_writer_release(w->block_writer);
    - 		w->block_writer = NULL;
    - 	}
    -+	strbuf_release(&w->last_key);
    - 	reftable_free(w->block);
    - 	reftable_free(w);
    - }
     @@ reftable/writer.c: static int writer_finish_section(struct reftable_writer *w)
      	bstats->max_index_level = max_level;
      
 7:  86dab54dfe ! 10:  26f422703f reftable/block: reuse zstream when writing log blocks
    @@ reftable/block.c: int block_writer_finish(struct block_writer *w)
     +		w->zstream->avail_in = src_len;
     +
     +		/*
    -+		 * We want to perform all decompression in a single
    -+		 * step, which is why we can pass Z_FINISH here. Note
    -+		 * that both `Z_OK` and `Z_BUF_ERROR` indicate that we
    -+		 * need to retry according to documentation.
    -+		 *
    -+		 * If the call fails we retry with a bigger output
    -+		 * buffer.
    ++		 * We want to perform all decompression in a single step, which
    ++		 * is why we can pass Z_FINISH here. As we have precomputed the
    ++		 * deflated buffer's size via `deflateBound()` this function is
    ++		 * guaranteed to succeed according to the zlib documentation.
     +		 */
     +		ret = deflate(w->zstream, Z_FINISH);
     +		if (ret != Z_STREAM_END) {
    @@ reftable/block.h: license that can be found in the LICENSE file or at
      	uint8_t *buf;
      	uint32_t block_size;
      
    -
    - ## reftable/writer.c ##
    -@@ reftable/writer.c: void reftable_writer_free(struct reftable_writer *w)
    - {
    - 	if (!w)
    - 		return;
    -+	if (w->block_writer) {
    -+		block_writer_release(w->block_writer);
    -+		w->block_writer = NULL;
    -+	}
    - 	reftable_free(w->block);
    - 	reftable_free(w);
    - }
 8:  9899b58dcf ! 11:  4f9df714da reftable/block: reuse compressed array
    @@ reftable/block.c: int block_writer_finish(struct block_writer *w)
      		w->zstream->next_in = w->buf + block_header_skip;
      		w->zstream->avail_in = src_len;
     @@ reftable/block.c: int block_writer_finish(struct block_writer *w)
    - 		 * buffer.
    + 		 * guaranteed to succeed according to the zlib documentation.
      		 */
      		ret = deflate(w->zstream, Z_FINISH);
     -		if (ret != Z_STREAM_END) {