git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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)?

Previous: Elijah NewrenNext: Elijah Newren
Message 11 of 18 in “range-diff: add configurable memory limit for cost matrix”
  1. range-diff: add configurable memory limit for cost matrixPaulo Casaretto via GitGitGadget, Aug 26, 2025
  2. Junio C HamanoAug 26, 2025
  3. 0/2 range-diff: add configurable memory limit for cost matrixPaulo Casaretto via GitGitGadget, Aug 28, 2025
  4. 1/2 range-diff: reorder options lexicographicallypcasaretto via GitGitGadget, Aug 28, 2025
  5. Junio C HamanoAug 28, 2025
  6. Elijah NewrenAug 28, 2025
  7. Paulo L F CasarettoAug 29, 2025
  8. Junio C HamanoAug 29, 2025
  9. 2/2 range-diff: add configurable memory limit for cost matrixpcasaretto via GitGitGadget, Aug 28, 2025
  10. Elijah NewrenAug 28, 2025
  11. Junio C HamanoAug 28, 2025
  12. Elijah NewrenAug 28, 2025
  13. Junio C HamanoAug 28, 2025
  14. range-diff: add configurable memory limit for cost matrixPaulo Casaretto via GitGitGadget, Aug 29, 2025
  15. Elijah NewrenAug 29, 2025
  16. Junio C HamanoAug 29, 2025
  17. Junio C HamanoAug 29, 2025
  18. range-diff: add configurable memory limit for cost matrixPaulo Casaretto via GitGitGadget, Aug 29, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.