git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Matheus Tavares <matheus.bernardino@usp.br>
To: git@vger.kernel.org
Cc: christian.couder@gmail.com, git@jeffhostetler.com,
	stolee@gmail.com, tboegi@web.de
Subject: [PATCH v3 0/8] Parallel Checkout (part 3)
Date: Tue,  4 May 2021 13:27:27 -0300	[thread overview]
Message-ID: <cover.1620145501.git.matheus.bernardino@usp.br> (raw)
In-Reply-To: <cover.1619818517.git.matheus.bernardino@usp.br>

This is the last part of the parallel checkout series. It adds tests and
parallel checkout support to `git checkout-index` and
`git checkout <pathspec>`.

I rebased this version onto `master`, as part-2 was merged down to
`master`.

Changes since v2:

* Patch 1: Aligned function parameters in make_transient_cache_entry(),
  and renamed the `struct mem_pool *` argument from `mp` to
  `ce_mem_pool`. This is more consistent with the other functions using
  in read-cache.c and with how we call this function in the next patch.

* Patch 2: Removed unnecessary 'Note: ' from commit message.

* Patch 3: Rewrote commit message to better explain why we unified the
  exit paths for `checkout_all()` and `checkout_file()` modes, and
  changed `git checkout-index --all`s error code from 128 to 1.

Matheus Tavares (8):
  make_transient_cache_entry(): optionally alloc from mem_pool
  builtin/checkout.c: complete parallel checkout support
  checkout-index: add parallel checkout support
  parallel-checkout: add tests for basic operations
  parallel-checkout: add tests related to path collisions
  t0028: extract encoding helpers to lib-encoding.sh
  parallel-checkout: add tests related to .gitattributes
  ci: run test round with parallel-checkout enabled

 builtin/checkout--worker.c              |   2 +-
 builtin/checkout-index.c                |  24 ++-
 builtin/checkout.c                      |  20 ++-
 builtin/difftool.c                      |   2 +-
 cache.h                                 |  14 +-
 ci/run-build-and-tests.sh               |   1 +
 parallel-checkout.c                     |  18 ++
 read-cache.c                            |  14 +-
 t/README                                |   4 +
 t/lib-encoding.sh                       |  25 +++
 t/lib-parallel-checkout.sh              |  45 +++++
 t/t0028-working-tree-encoding.sh        |  25 +--
 t/t2080-parallel-checkout-basics.sh     | 229 ++++++++++++++++++++++++
 t/t2081-parallel-checkout-collisions.sh | 162 +++++++++++++++++
 t/t2082-parallel-checkout-attributes.sh | 194 ++++++++++++++++++++
 unpack-trees.c                          |   2 +-
 16 files changed, 732 insertions(+), 49 deletions(-)
 create mode 100644 t/lib-encoding.sh
 create mode 100644 t/lib-parallel-checkout.sh
 create mode 100755 t/t2080-parallel-checkout-basics.sh
 create mode 100755 t/t2081-parallel-checkout-collisions.sh
 create mode 100755 t/t2082-parallel-checkout-attributes.sh

Range-diff against v2:
1:  f870040bfb ! 1:  bf6c7114aa make_transient_cache_entry(): optionally alloc from mem_pool
    @@ cache.h: struct cache_entry *make_empty_cache_entry(struct index_state *istate,
     - * Create a cache_entry that is not intended to be added to an index.
     - * Caller is responsible for discarding the cache_entry
     - * with `discard_cache_entry`.
    -+ * Create a cache_entry that is not intended to be added to an index. If `mp`
    -+ * is not NULL, the entry is allocated within the given memory pool. Caller is
    -+ * responsible for discarding "loose" entries with `discard_cache_entry()` and
    -+ * the mem_pool with `mem_pool_discard(mp, should_validate_cache_entries())`.
    ++ * Create a cache_entry that is not intended to be added to an index. If
    ++ * `ce_mem_pool` is not NULL, the entry is allocated within the given memory
    ++ * pool. Caller is responsible for discarding "loose" entries with
    ++ * `discard_cache_entry()` and the memory pool with
    ++ * `mem_pool_discard(ce_mem_pool, should_validate_cache_entries())`.
       */
      struct cache_entry *make_transient_cache_entry(unsigned int mode,
      					       const struct object_id *oid,
      					       const char *path,
     -					       int stage);
    -+					       int stage, struct mem_pool *mp);
    ++					       int stage,
    ++					       struct mem_pool *ce_mem_pool);
      
     -struct cache_entry *make_empty_transient_cache_entry(size_t name_len);
    -+struct cache_entry *make_empty_transient_cache_entry(size_t len, struct mem_pool *mp);
    ++struct cache_entry *make_empty_transient_cache_entry(size_t len,
    ++						     struct mem_pool *ce_mem_pool);
      
      /*
       * Discard cache entry.
    @@ read-cache.c: struct cache_entry *make_empty_cache_entry(struct index_state *ist
      }
      
     -struct cache_entry *make_empty_transient_cache_entry(size_t len)
    -+struct cache_entry *make_empty_transient_cache_entry(size_t len, struct mem_pool *mp)
    ++struct cache_entry *make_empty_transient_cache_entry(size_t len,
    ++						     struct mem_pool *ce_mem_pool)
      {
    -+	if (mp)
    -+		return mem_pool__ce_calloc(mp, len);
    ++	if (ce_mem_pool)
    ++		return mem_pool__ce_calloc(ce_mem_pool, len);
      	return xcalloc(1, cache_entry_size(len));
      }
      
    @@ read-cache.c: struct cache_entry *make_cache_entry(struct index_state *istate,
     -					       const char *path, int stage)
     +struct cache_entry *make_transient_cache_entry(unsigned int mode,
     +					       const struct object_id *oid,
    -+					       const char *path, int stage,
    -+					       struct mem_pool *mp)
    ++					       const char *path,
    ++					       int stage,
    ++					       struct mem_pool *ce_mem_pool)
      {
      	struct cache_entry *ce;
      	int len;
    @@ read-cache.c: struct cache_entry *make_transient_cache_entry(unsigned int mode,
      
      	len = strlen(path);
     -	ce = make_empty_transient_cache_entry(len);
    -+	ce = make_empty_transient_cache_entry(len, mp);
    ++	ce = make_empty_transient_cache_entry(len, ce_mem_pool);
      
      	oidcpy(&ce->oid, oid);
      	memcpy(ce->name, path, len);
2:  e2d82c4337 ! 2:  e898889787 builtin/checkout.c: complete parallel checkout support
    @@ Commit message
         checkout_entry() directly, instead of unpack_trees(). Let's add parallel
         checkout support for this code path too.
     
    -    Note: the transient cache entries allocated in checkout_merged() are now
    +    The transient cache entries allocated in checkout_merged() are now
         allocated in a mem_pool which is only discarded after parallel checkout
         finishes. This is done because the entries need to be valid when
         run_parallel_checkout() is called.
    @@ builtin/checkout.c: static int checkout_worktree(const struct checkout_opts *opt
      	enable_delayed_checkout(&state);
     +	if (pc_workers > 1)
     +		init_parallel_checkout();
    - 	for (pos = 0; pos < active_nr; pos++) {
    - 		struct cache_entry *ce = active_cache[pos];
    - 		if (ce->ce_flags & CE_MATCHED) {
    + 
    + 	/* TODO: audit for interaction with sparse-index. */
    + 	ensure_full_index(&the_index);
     @@ builtin/checkout.c: static int checkout_worktree(const struct checkout_opts *opts,
      						       &nr_checkouts, opts->overlay_mode);
      			else if (opts->merge)
3:  0fe1a5fabc ! 3:  916f391a46 checkout-index: add parallel checkout support
    @@ Metadata
      ## Commit message ##
         checkout-index: add parallel checkout support
     
    -    Note: previously, `checkout_all()` would not return on errors, but
    -    instead call `exit()` with a non-zero code. However, it only did that
    -    after calling `checkout_entry()` for all index entries, thus not
    -    stopping on the first error, but attempting to write the maximum number
    -    of entries possible. In order to maintain this behavior we now propagate
    -    `checkout_all()`s error status to `cmd_checkout_index()`, so that it can
    -    call `run_parallel_checkout()` and attempt to write the queued entries
    -    before exiting with the error code.
    +    Allow checkout-index to use the parallel checkout framework, honoring
    +    the checkout.workers configuration.
    +
    +    There are two code paths in checkout-index which call
    +    `checkout_entry()`, and thus, can make use of parallel checkout:
    +    `checkout_file()`, which is used to write paths explicitly given at the
    +    command line; and `checkout_all()`, which is used to write all paths in
    +    the index, when the `--all` option is given.
    +
    +    In both operation modes, checkout-index doesn't abort immediately on a
    +    `checkout_entry()` failure. Instead, it tries to check out all remaining
    +    paths before exiting with a non-zero exit code. To keep this behavior
    +    when parallel checkout is being used, we must allow
    +    `run_parallel_checkout()` to try writing the queued entries before we
    +    exit, even if we already got an error code from a previous
    +    `checkout_entry()` call.
    +
    +    However, `checkout_all()` doesn't return on errors, it calls `exit()`
    +    with code 128. We could make it call `run_parallel_checkout()` before
    +    exiting, but it makes the code easier to follow if we unify the exit
    +    path for both checkout-index modes at `cmd_checkout_index()`, and let
    +    this function take care of the interactions with the parallel checkout
    +    API. So let's do that.
    +
    +    With this change, we also have to consider whether we want to keep using
    +    128 as the error code for `git checkout-index --all`, while we use 1 for
    +    `git checkout-index <path>` (even when the actual error is the same).
    +    Since there is not much value in having code 128 only for `--all`, and
    +    there is no mention about it in the docs (so it's unlikely that changing
    +    it will break any existing script), let's make both modes exit with code
    +    1 on `checkout_entry()` errors.
     
         Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
     
4:  f604c50dba = 4:  667777053a parallel-checkout: add tests for basic operations
5:  eae6d3a1c1 = 5:  dcb3acab1d parallel-checkout: add tests related to path collisions
6:  9161cd1503 = 6:  6141c46051 t0028: extract encoding helpers to lib-encoding.sh
7:  bc584897e6 = 7:  5350689a30 parallel-checkout: add tests related to .gitattributes
8:  1bc5c523c5 ! 8:  7b2966c488 ci: run test round with parallel-checkout enabled
    @@ parallel-checkout.c: static const int DEFAULT_NUM_WORKERS = 1;
      	else if (*num_workers < 1)
     
      ## t/README ##
    -@@ t/README: and "sha256".
    - GIT_TEST_WRITE_REV_INDEX=<boolean>, when true enables the
    - 'pack.writeReverseIndex' setting.
    +@@ t/README: GIT_TEST_WRITE_REV_INDEX=<boolean>, when true enables the
    + GIT_TEST_SPARSE_INDEX=<boolean>, when true enables index writes to use the
    + sparse-index format by default.
      
     +GIT_TEST_CHECKOUT_WORKERS=<n> overrides the 'checkout.workers' setting
     +to <n> and 'checkout.thresholdForParallelism' to 0, forcing the
-- 
2.30.1


  parent reply	other threads:[~2021-05-04 16:27 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-22 15:17 [PATCH 0/7] " Matheus Tavares
2021-04-22 15:17 ` [PATCH 1/7] make_transient_cache_entry(): optionally alloc from mem_pool Matheus Tavares
2021-04-22 15:17 ` [PATCH 2/7] builtin/checkout.c: complete parallel checkout support Matheus Tavares
2021-04-23 16:19   ` Derrick Stolee
2021-04-26 21:54     ` Matheus Tavares Bernardino
2021-04-22 15:17 ` [PATCH 3/7] checkout-index: add " Matheus Tavares
2021-04-23 18:32   ` Derrick Stolee
2021-04-26 22:30     ` Matheus Tavares Bernardino
2021-04-22 15:17 ` [PATCH 4/7] parallel-checkout: add tests for basic operations Matheus Tavares
2021-04-23 19:18   ` Derrick Stolee
2021-04-27  2:30     ` Matheus Tavares Bernardino
2021-04-22 15:17 ` [PATCH 5/7] parallel-checkout: add tests related to path collisions Matheus Tavares
2021-04-22 15:17 ` [PATCH 6/7] parallel-checkout: add tests related to .gitattributes Matheus Tavares
2021-04-23 19:48   ` Derrick Stolee
2021-04-22 15:17 ` [PATCH 7/7] ci: run test round with parallel-checkout enabled Matheus Tavares
2021-04-23 19:56   ` Derrick Stolee
2021-04-30 21:40 ` [PATCH v2 0/8] Parallel Checkout (part 3) Matheus Tavares
2021-04-30 21:40   ` [PATCH v2 1/8] make_transient_cache_entry(): optionally alloc from mem_pool Matheus Tavares
2021-05-01 17:06     ` Christian Couder
2021-05-03 14:11       ` Matheus Tavares Bernardino
2021-04-30 21:40   ` [PATCH v2 2/8] builtin/checkout.c: complete parallel checkout support Matheus Tavares
2021-05-01 17:08     ` Christian Couder
2021-05-03 14:21       ` Matheus Tavares Bernardino
2021-04-30 21:40   ` [PATCH v2 3/8] checkout-index: add " Matheus Tavares
2021-05-01 17:08     ` Christian Couder
2021-05-03 14:22       ` Matheus Tavares Bernardino
2021-04-30 21:40   ` [PATCH v2 4/8] parallel-checkout: add tests for basic operations Matheus Tavares
2021-04-30 21:40   ` [PATCH v2 5/8] parallel-checkout: add tests related to path collisions Matheus Tavares
2021-05-02  7:59     ` Torsten Bögershausen
2021-05-03 14:58       ` Matheus Tavares Bernardino
2021-04-30 21:40   ` [PATCH v2 6/8] t0028: extract encoding helpers to lib-encoding.sh Matheus Tavares
2021-04-30 21:40   ` [PATCH v2 7/8] parallel-checkout: add tests related to .gitattributes Matheus Tavares
2021-04-30 21:40   ` [PATCH v2 8/8] ci: run test round with parallel-checkout enabled Matheus Tavares
2021-05-02 10:12   ` [PATCH v2 0/8] Parallel Checkout (part 3) Torsten Bögershausen
2021-05-03 15:01     ` Matheus Tavares Bernardino
2021-05-04 16:27   ` Matheus Tavares [this message]
2021-05-04 16:27     ` [PATCH v3 1/8] make_transient_cache_entry(): optionally alloc from mem_pool Matheus Tavares
2021-05-04 16:27     ` [PATCH v3 2/8] builtin/checkout.c: complete parallel checkout support Matheus Tavares
2021-05-05 13:55       ` Derrick Stolee
2021-05-04 16:27     ` [PATCH v3 3/8] checkout-index: add " Matheus Tavares
2021-05-04 16:27     ` [PATCH v3 4/8] parallel-checkout: add tests for basic operations Matheus Tavares
2021-05-26 18:36       ` AIX failures on parallel checkout (new in v2.32.0-rc*) Ævar Arnfjörð Bjarmason
2021-05-26 22:01         ` Matheus Tavares Bernardino
2021-05-26 23:00           ` Junio C Hamano
2021-05-04 16:27     ` [PATCH v3 5/8] parallel-checkout: add tests related to path collisions Matheus Tavares
2021-05-04 16:27     ` [PATCH v3 6/8] t0028: extract encoding helpers to lib-encoding.sh Matheus Tavares
2021-05-04 16:27     ` [PATCH v3 7/8] parallel-checkout: add tests related to .gitattributes Matheus Tavares
2021-05-04 16:27     ` [PATCH v3 8/8] ci: run test round with parallel-checkout enabled Matheus Tavares
2021-05-05 13:57     ` [PATCH v3 0/8] Parallel Checkout (part 3) Derrick Stolee
2021-05-06  0:40       ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1620145501.git.matheus.bernardino@usp.br \
    --to=matheus.bernardino@usp.br \
    --cc=christian.couder@gmail.com \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=stolee@gmail.com \
    --cc=tboegi@web.de \
    --subject='Re: [PATCH v3 0/8] Parallel Checkout (part 3)' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git