Subject: [PATCH v3 00/11] reftable: optimize write performance

From: Patrick Steinhardt <[email-masked]>
Date: Mon, 8 Apr 2024 14:23:47 +0200

Hi,

this is the first version of my patch series that aims to optimize write
performance with the reftable backend.

Changes compared to v2:

    - The series now deepends on ps/reftable-binsearch-update at
      d51d8cc368 (reftable/block: avoid decoding keys when searching
      restart points, 2024-04-03). This is to resolve a merge conflict
      with that other series which has landed in "next" already.

    - Dropped the "reftable_" prefix from newly introduced internal
      reftable functions.

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         | 206 -------------------------------------
 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(+), 556 deletions(-)
 delete mode 100644 reftable/refname.c
 delete mode 100644 reftable/refname.h
 delete mode 100644 reftable/refname_test.c

Range-diff against v2:
 1:  926e802395 =  1:  bb735c389a refs/reftable: fix D/F conflict error message on ref copy
 2:  6190171906 =  2:  fe3f00d85a refs/reftable: perform explicit D/F check when writing symrefs
 3:  80008cc5e7 =  3:  763c6fdfcd refs/reftable: skip duplicate name checks
 4:  3497a570b4 !  4:  2a5f07627a reftable: remove name checks
    @@ reftable/refname.c (deleted)
     -#include "refname.h"
     -#include "reftable-iterator.h"
     -
    --struct find_arg {
    --	char **names;
    --	const char *want;
    +-struct refname_needle_lesseq_args {
    +-	char **haystack;
    +-	const char *needle;
     -};
     -
    --static int find_name(size_t k, void *arg)
    +-static int refname_needle_lesseq(size_t k, void *_args)
     -{
    --	struct find_arg *f_arg = arg;
    --	return strcmp(f_arg->names[k], f_arg->want) >= 0;
    +-	struct refname_needle_lesseq_args *args = _args;
    +-	return strcmp(args->needle, args->haystack[k]) <= 0;
     -}
     -
     -static int modification_has_ref(struct modification *mod, const char *name)
    @@ reftable/refname.c (deleted)
     -	int err = 0;
     -
     -	if (mod->add_len > 0) {
    --		struct find_arg arg = {
    --			.names = mod->add,
    --			.want = name,
    +-		struct refname_needle_lesseq_args args = {
    +-			.haystack = mod->add,
    +-			.needle = name,
     -		};
    --		int idx = binsearch(mod->add_len, find_name, &arg);
    --		if (idx < mod->add_len && !strcmp(mod->add[idx], name)) {
    +-		size_t idx = binsearch(mod->add_len, refname_needle_lesseq, &args);
    +-		if (idx < mod->add_len && !strcmp(mod->add[idx], name))
     -			return 0;
    --		}
     -	}
     -
     -	if (mod->del_len > 0) {
    --		struct find_arg arg = {
    --			.names = mod->del,
    --			.want = name,
    +-		struct refname_needle_lesseq_args args = {
    +-			.haystack = mod->del,
    +-			.needle = name,
     -		};
    --		int idx = binsearch(mod->del_len, find_name, &arg);
    --		if (idx < mod->del_len && !strcmp(mod->del[idx], name)) {
    +-		size_t idx = binsearch(mod->del_len, refname_needle_lesseq, &args);
    +-		if (idx < mod->del_len && !strcmp(mod->del[idx], name))
     -			return 1;
    --		}
     -	}
     -
     -	err = reftable_table_read_ref(&mod->tab, name, &ref);
    @@ reftable/refname.c (deleted)
     -	int err = 0;
     -
     -	if (mod->add_len > 0) {
    --		struct find_arg arg = {
    --			.names = mod->add,
    --			.want = prefix,
    +-		struct refname_needle_lesseq_args args = {
    +-			.haystack = mod->add,
    +-			.needle = prefix,
     -		};
    --		int idx = binsearch(mod->add_len, find_name, &arg);
    +-		size_t idx = binsearch(mod->add_len, refname_needle_lesseq, &args);
     -		if (idx < mod->add_len &&
     -		    !strncmp(prefix, mod->add[idx], strlen(prefix)))
     -			goto done;
    @@ reftable/refname.c (deleted)
     -			goto done;
     -
     -		if (mod->del_len > 0) {
    --			struct find_arg arg = {
    --				.names = mod->del,
    --				.want = ref.refname,
    +-			struct refname_needle_lesseq_args args = {
    +-				.haystack = mod->del,
    +-				.needle = ref.refname,
     -			};
    --			int idx = binsearch(mod->del_len, find_name, &arg);
    +-			size_t idx = binsearch(mod->del_len, refname_needle_lesseq, &args);
     -			if (idx < mod->del_len &&
    --			    !strcmp(ref.refname, mod->del[idx])) {
    +-			    !strcmp(ref.refname, mod->del[idx]))
     -				continue;
    --			}
     -		}
     -
     -		if (strncmp(ref.refname, prefix, strlen(prefix))) {
 5:  f892a3007b =  5:  1ca7d9b6cf refs/reftable: don't recompute committer ident
 6:  4877ab3921 =  6:  deabf82186 reftable/writer: refactorings for `writer_add_record()`
 7:  8f1c5b4169 =  7:  d47ad49d49 reftable/writer: refactorings for `writer_flush_nonempty_block()`
 8:  41db7414e1 !  8:  76d4a1f73b reftable/writer: unify releasing memory
    @@ reftable/writer.c: void reftable_writer_set_limits(struct reftable_writer *w, ui
      	w->max_update_index = max;
      }
      
    -+static void reftable_writer_release(struct reftable_writer *w)
    ++static void writer_release(struct reftable_writer *w)
     +{
     +	if (w) {
     +		reftable_free(w->block);
    @@ reftable/writer.c: void reftable_writer_set_limits(struct reftable_writer *w, ui
     -	if (!w)
     -		return;
     -	reftable_free(w->block);
    -+	reftable_writer_release(w);
    ++	writer_release(w);
      	reftable_free(w);
      }
      
    @@ reftable/writer.c: int reftable_writer_close(struct reftable_writer *w)
     -	block_writer_release(&w->block_writer_data);
     -	writer_clear_index(w);
     -	strbuf_release(&w->last_key);
    -+	reftable_writer_release(w);
    ++	writer_release(w);
      	return err;
      }
      
 9:  e5c7dbe417 =  9:  722ab0ee28 reftable/writer: reset `last_key` instead of releasing it
10:  26f422703f = 10:  962a96003b reftable/block: reuse zstream when writing log blocks
11:  4f9df714da = 11:  323892841a reftable/block: reuse compressed array

base-commit: 7774cfed6261ce2900c84e55906da708c711d601
-- 
2.44.GIT