Re: [PATCH v2 2/2] range-diff: add configurable memory limit for cost matrix
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 28, 2025, 21:22 UTC
- Message-ID
- <xmqqiki7ta3e.fsf@gitster.g>
- In-Reply-To
- <CABPp-BEDje5dYZHEyYMN6j_LdR5CqRN1cxc0riRK06qK-OxiTA@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
Show 37 quoted lines
> <gitgitgadget@gmail.com> wrote:
>>
>> From: pcasaretto <paulo.casaretto@shopify.com>
>> Signed-off-by: Paulo Casaretto <paulo.casaretto@shopify.com>
>
> The names (and emails) in these should match; I believe the name in
> the From field is set by Gitgitgadget based on your profile settings;
> see https://github.com/settings/profile and set your name there.
>
>> static void get_correspondences(struct string_list *a, struct string_list *b,
>> - int creation_factor)
>> + int creation_factor, size_t max_memory)
>> {
>> int n = a->nr + b->nr;
>> int *cost, c, *a2b, *b2a;
>> int i, j;
>> -
>> - ALLOC_ARRAY(cost, st_mult(n, n));
>> + size_t cost_size = st_mult(n, n);
>> + size_t cost_bytes = st_mult(sizeof(int), cost_size);
>> + if (cost_bytes >= max_memory) {
>> + struct strbuf cost_str = STRBUF_INIT;
>> + struct strbuf max_str = STRBUF_INIT;
>> + strbuf_humanise_bytes(&cost_str, cost_bytes);
>> + strbuf_humanise_bytes(&max_str, max_memory);
>> + die(_("range-diff: unable to compute the range-diff, since it "
>> + "exceeds the maximum memory for the cost matrix: %s "
>> + "(%"PRIuMAX" bytes) needed, %s (%"PRIuMAX" bytes) available"),
>
> available? I'm worried the error message will report in users
> checking system memory, claiming they have 14GB available on their
> system, and then reporting a "bug".
>
> Perhaps something like:
>
> + "(%"PRIuMAX" bytes) needed, limited to %s
> (%"PRIuMAX" bytes)"),Sounds like a good idea.
I am not a huge fan of configuration variables that do not have a command line option. Assuming that it is not like you'd be doing overly huge range-diff that would not fit your memory every day, shouldn't we start this with a command line option without a configuration variable to gauge how useful it would be for users with such a need, and then after it proves useful and we identify a workflow where a user would be passing this option all the time, add a configuration to allow it always be in effect (with command line override still available)?