[v3,0/5] Parallel Checkout (part 2)

Message

Matheus Tavares April 19, 2021, 12:14 a.m. UTC

Three small changes since the last version: 

- In `write_pc_item_to_fd()`, renamed variable 'new_blob' to just 'blob'.

- In `write_pc_item_to_fd()`, used `ce->name` instead of `path` when
  reporting failure to read the object.

- Made `write_pc_item()` unlink() the file on a `write_pc_item_to_fd()`
  error. The reasoning for this is:

  Unlike sequential checkout, the parallel workers create each file
  *before* reading and converting the associated blob, so that they can
  detect path collisions as soon as possible in the process. This is
  important both to avoid unnecessary work reading and filtering
  objects that are not going to be written in parallel, and also to
  avoid reporting errors for colliding entries at the wrong time. These
  entries will be handled in a sequential phase later, and that will be
  the correct time to print any errors regarding their checkout.

  However, these logistics make us leave an empty file behind when we
  find an error after creating the file, e.g. a missing object. The
  added unlink() fix this case while maintaining the other important
  mechanics. The design doc was also adjusted to mention this.

Matheus Tavares (5):
  unpack-trees: add basic support for parallel checkout
  parallel-checkout: make it truly parallel
  parallel-checkout: add configuration options
  parallel-checkout: support progress displaying
  parallel-checkout: add design documentation

 .gitignore                                    |   1 +
 Documentation/Makefile                        |   1 +
 Documentation/config/checkout.txt             |  21 +
 Documentation/technical/parallel-checkout.txt | 271 ++++++++
 Makefile                                      |   2 +
 builtin.h                                     |   1 +
 builtin/checkout--worker.c                    | 145 ++++
 entry.c                                       |  17 +-
 git.c                                         |   2 +
 parallel-checkout.c                           | 655 ++++++++++++++++++
 parallel-checkout.h                           | 111 +++
 unpack-trees.c                                |  19 +-
 12 files changed, 1241 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/technical/parallel-checkout.txt
 create mode 100644 builtin/checkout--worker.c
 create mode 100644 parallel-checkout.c
 create mode 100644 parallel-checkout.h

Range-diff against v2:
1:  0506ac7159 ! 1:  7096822c14 unpack-trees: add basic support for parallel checkout
    @@ parallel-checkout.c (new)
     +	int ret;
     +	struct stream_filter *filter;
     +	struct strbuf buf = STRBUF_INIT;
    -+	char *new_blob;
    ++	char *blob;
     +	unsigned long size;
     +	ssize_t wrote;
     +
    @@ parallel-checkout.c (new)
     +		}
     +	}
     +
    -+	new_blob = read_blob_entry(pc_item->ce, &size);
    -+	if (!new_blob)
    ++	blob = read_blob_entry(pc_item->ce, &size);
    ++	if (!blob)
     +		return error("cannot read object %s '%s'",
    -+			     oid_to_hex(&pc_item->ce->oid), path);
    ++			     oid_to_hex(&pc_item->ce->oid), pc_item->ce->name);
     +
     +	/*
     +	 * checkout metadata is used to give context for external process
    @@ parallel-checkout.c (new)
     +	 * checkout, so pass NULL.
     +	 */
     +	ret = convert_to_working_tree_ca(&pc_item->ca, pc_item->ce->name,
    -+					 new_blob, size, &buf, NULL);
    ++					 blob, size, &buf, NULL);
     +
     +	if (ret) {
     +		size_t newsize;
    -+		free(new_blob);
    -+		new_blob = strbuf_detach(&buf, &newsize);
    ++		free(blob);
    ++		blob = strbuf_detach(&buf, &newsize);
     +		size = newsize;
     +	}
     +
    -+	wrote = write_in_full(fd, new_blob, size);
    -+	free(new_blob);
    ++	wrote = write_in_full(fd, blob, size);
    ++	free(blob);
     +	if (wrote < 0)
     +		return error("unable to write file '%s'", path);
     +
    @@ parallel-checkout.c (new)
     +	if (write_pc_item_to_fd(pc_item, fd, path.buf)) {
     +		/* Error was already reported. */
     +		pc_item->status = PC_ITEM_FAILED;
    ++		close_and_clear(&fd);
    ++		unlink(path.buf);
     +		goto out;
     +	}
     +
    @@ parallel-checkout.c (new)
     +	pc_item->status = PC_ITEM_WRITTEN;
     +
     +out:
    -+	/*
    -+	 * No need to check close() return at this point. Either fd is already
    -+	 * closed, or we are on an error path.
    -+	 */
    -+	close_and_clear(&fd);
     +	strbuf_release(&path);
     +}
     +
2:  0d65b517bd ! 2:  4526516ea0 parallel-checkout: make it truly parallel
    @@ parallel-checkout.c: static int write_pc_item_to_fd(struct parallel_checkout_ite
     +	 * be passed from the main process to the workers.
      	 */
      	ret = convert_to_working_tree_ca(&pc_item->ca, pc_item->ce->name,
    - 					 new_blob, size, &buf, NULL);
    + 					 blob, size, &buf, NULL);
     @@ parallel-checkout.c: static int close_and_clear(int *fd)
      	return ret;
      }
3:  6ea057f9c5 = 3:  ad165c0637 parallel-checkout: add configuration options
4:  0ac4bee67e = 4:  cf9e28dc0e parallel-checkout: support progress displaying
5:  087f8bdf35 ! 5:  415d4114aa parallel-checkout: add design documentation
    @@ Documentation/technical/parallel-checkout.txt (new)
     +Note that, when possible, steps W3 to W5 are delegated to the streaming
     +machinery, removing the need to keep the entire blob in memory.
     +
    -+Also note that the workers *never* remove any file. As mentioned
    -+earlier, it is the responsibility of the main process to remove any file
    -+that blocks the checkout operation (or abort if removing the file(s)
    -+would cause data loss and the user didn't ask to `--force`). This is
    -+crucial to avoid race conditions and also to properly detect path
    -+collisions at Step W1.
    ++If the worker fails to read the blob or to write it to the working tree,
    ++it removes the created file to avoid leaving empty files behind. This is
    ++the *only* time a worker is allowed to remove a file.
    ++
    ++As mentioned earlier, it is the responsibility of the main process to
    ++remove any file that blocks the checkout operation (or abort if the
    ++removal(s) would cause data loss and the user didn't ask to `--force`).
    ++This is crucial to avoid race conditions and also to properly detect
    ++path collisions at Step W1.
     +
     +After the workers finish writing the items and sending back the required
     +information, the main process handles the results in two steps: