Re: [PATCH v2 10/23] pack-bitmap-write.c: select pseudo-merge commits

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 3027 bytes --]

On Mon, Apr 29, 2024 at 04:43:41PM -0400, Taylor Blau wrote:
[snip]
> diff --git a/Documentation/config/bitmap-pseudo-merge.txt b/Documentation/config/bitmap-pseudo-merge.txt
> new file mode 100644
> index 00000000000..90b72522046
> --- /dev/null
> +++ b/Documentation/config/bitmap-pseudo-merge.txt
> @@ -0,0 +1,75 @@
> +bitmapPseudoMerge.<name>.pattern::
> +	Regular expression used to match reference names. Commits
> +	pointed to by references matching this pattern (and meeting
> +	the below criteria, like `bitmapPseudoMerge.<name>.sampleRate`
> +	and `bitmapPseudoMerge.<name>.threshold`) will be considered
> +	for inclusion in a pseudo-merge bitmap.
> ++
> +Commits are grouped into pseudo-merge groups based on whether or not
> +any reference(s) that point at a given commit match the pattern, which
> +is an extended regular expression.
> ++
> +Within a pseudo-merge group, commits may be further grouped into
> +sub-groups based on the capture groups in the pattern. These
> +sub-groupings are formed from the regular expressions by concatenating
> +any capture groups from the regular expression, with a '-' dash in
> +between.
> ++
> +For example, if the pattern is `refs/tags/`, then all tags (provided
> +they meet the below criteria) will be considered candidates for the
> +same pseudo-merge group. However, if the pattern is instead
> +`refs/remotes/([0-9])+/tags/`, then tags from different remotes will
> +be grouped into separate pseudo-merge groups, based on the remote
> +number.
> +
> +bitmapPseudoMerge.<name>.decay::
> +	Determines the rate at which consecutive pseudo-merge bitmap
> +	groups decrease in size. Must be non-negative. This parameter
> +	can be thought of as `k` in the function `f(n) = C *
> +	n^(-k/100)`, where `f(n)` is the size of the `n`th group.
> ++
> +Setting the decay rate equal to `0` will cause all groups to be the
> +same size. Setting the decay rate equal to `100` will cause the `n`th
> +group to be `1/n` the size of the initial group.  Higher values of the
> +decay rate cause consecutive groups to shrink at an increasing rate.
> +The default is `100`.
> +
> +bitmapPseudoMerge.<name>.sampleRate::
> +	Determines the proportion of non-bitmapped commits (among
> +	reference tips) which are selected for inclusion in an
> +	unstable pseudo-merge bitmap. Must be between `0` and `100`
> +	(inclusive). The default is `100`.

I think for this config to be actionable for anybody we need to explain
what "unstable" or "stable" bitmaps are and what the tradeoff is that
the user needs to pick here. Like, why would I want to set this higher
or lower than the default value, or modify it at all?

I think the same is true for most of the other parts of the docs here,
as well. We explain what those configs do, but basically leave the
reader on their own to figure out what the real-world consequences are
and why they would even want to configure those in the first place.

I spent quite some time on this series now, so I'll stop reading at this
point. Thanks!

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]