Re: [RFC PATCH 2/5] Add delta-islands.{c,h}

[Duy Nguyen <pclouds@gmail.com>]

On Sun, Jul 22, 2018 at 7:51 AM Christian Couder
<christian.couder@gmail.com> wrote:
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index a32172a43c..f682e92a1a 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2542,6 +2542,10 @@ Note that changing the compression level will not automatically recompress
>  all existing objects. You can force recompression by passing the -F option
>  to linkgit:git-repack[1].
>
> +pack.island::
> +       A regular expression configuring a set of delta islands. See
> +       "DELTA ISLANDS" in linkgit:git-pack-objects[1] for details.
> +

That section is not added until 3/5 though.

> diff --git a/delta-islands.c b/delta-islands.c
> new file mode 100644
> index 0000000000..645fe966c5
> --- /dev/null
> +++ b/delta-islands.c
> @@ -0,0 +1,490 @@
> +#include "builtin.h"

A bit weird that builtin.h would be needed...

> +void resolve_tree_islands(int progress, struct packing_data *to_pack)
> +{
> +       struct progress *progress_state = NULL;
> +       struct object_entry **todo;
> +       int nr = 0;
> +       int i;
> +
> +       if (!island_marks)
> +               return;
> +
> +       /*
> +        * We process only trees, as commits and tags have already been handled
> +        * (and passed their marks on to root trees, as well. We must make sure
> +        * to process them in descending tree-depth order so that marks
> +        * propagate down the tree properly, even if a sub-tree is found in
> +        * multiple parent trees.
> +        */
> +       todo = xmalloc(to_pack->nr_objects * sizeof(*todo));
> +       for (i = 0; i < to_pack->nr_objects; i++) {
> +               if (oe_type(&to_pack->objects[i]) == OBJ_TREE)
> +                       todo[nr++] = &to_pack->objects[i];
> +       }
> +       qsort(todo, nr, sizeof(*todo), cmp_tree_depth);
> +
> +       if (progress)
> +               progress_state = start_progress("Propagating island marks", nr);

_() (same comment for other strings too)

> diff --git a/pack-objects.h b/pack-objects.h
> index edf74dabdd..8eecd67991 100644
> --- a/pack-objects.h
> +++ b/pack-objects.h
> @@ -100,6 +100,10 @@ struct object_entry {
>         unsigned type_:TYPE_BITS;
>         unsigned no_try_delta:1;
>         unsigned in_pack_type:TYPE_BITS; /* could be delta */
> +
> +       unsigned int tree_depth; /* should be repositioned for packing? */
> +       unsigned char layer;
> +

This looks very much like an optional feature. To avoid increasing
pack-objects memory usage for common case, please move this to struct
packing_data.

>         unsigned preferred_base:1; /*
>                                     * we do not pack this, but is available
>                                     * to be used as the base object to delta
> --
> 2.18.0.237.gffdb1dbdaa
-- 
Duy