Skip to content

Commit f6a1c6d

Browse files
committed
range-diff: add configurable memory limit for cost matrix
When comparing large commit ranges (e.g., 250,000+ commits), range-diff attempts to allocate an n×n cost matrix that can exhaust available memory. For example, with 256,784 commits (n = 513,568), the matrix would require approximately 256GB of memory (513,568² × 4 bytes), causing either immediate segmentation faults due to integer overflow or system hangs. Add a memory limit check in get_correspondences() before allocating the cost matrix. This check uses the total size in bytes (n² × sizeof(int)) and compares it against a configurable maximum, preventing both excessive memory usage and integer overflow issues. The limit is configurable via a new --max-memory option that accepts human-readable sizes (e.g., "1G", "500M"). The default is 4GB for 64 bit systems and 2GB for 32 bit systems. This allows comparing ranges of approximately 32,000 (16,000) commits - generous for real-world use cases while preventing impractical operations. When the limit is exceeded, range-diff now displays a clear error message showing both the requested memory size and the maximum allowed, formatted in human-readable units for better user experience. Example usage: git range-diff --max-memory=1G branch1...branch2 git range-diff --max-memory=500M base..topic1 base..topic2 This approach was chosen over alternatives: - Pre-counting commits: Would require spawning additional git processes and reading all commits twice - Limiting by commit count: Less precise than actual memory usage - Streaming approach: Would require significant refactoring of the current algorithm This issue was previously discussed in: https://lore.kernel.org/git/[email protected]/ Signed-off-by: pcasaretto <[email protected]>
1 parent 954d33a commit f6a1c6d

File tree

5 files changed

+54
-10
lines changed

5 files changed

+54
-10
lines changed

builtin/log.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1404,6 +1404,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
14041404
struct range_diff_options range_diff_opts = {
14051405
.creation_factor = rev->creation_factor,
14061406
.dual_color = 1,
1407+
.max_memory = RANGE_DIFF_MAX_MEMORY_DEFAULT,
14071408
.diffopt = &opts,
14081409
.other_arg = &other_arg
14091410
};

builtin/range-diff.c

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "parse-options.h"
77
#include "range-diff.h"
88
#include "config.h"
9+
#include "parse.h"
910

1011

1112
static const char * const builtin_range_diff_usage[] = {
@@ -15,6 +16,22 @@ N_("git range-diff [<options>] <base> <old-tip> <new-tip>"),
1516
NULL
1617
};
1718

19+
static int parse_max_memory(const struct option *opt, const char *arg, int unset)
20+
{
21+
size_t *max_memory = opt->value;
22+
uintmax_t val;
23+
24+
if (unset) {
25+
return 0;
26+
}
27+
28+
if (!git_parse_unsigned(arg, &val, SIZE_MAX))
29+
return error(_("invalid max-memory value: %s"), arg);
30+
31+
*max_memory = (size_t)val;
32+
return 0;
33+
}
34+
1835
int cmd_range_diff(int argc,
1936
const char **argv,
2037
const char *prefix,
@@ -25,6 +42,7 @@ int cmd_range_diff(int argc,
2542
struct strvec diff_merges_arg = STRVEC_INIT;
2643
struct range_diff_options range_diff_opts = {
2744
.creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT,
45+
.max_memory = RANGE_DIFF_MAX_MEMORY_DEFAULT,
2846
.diffopt = &diffopt,
2947
.other_arg = &other_arg
3048
};
@@ -33,6 +51,10 @@ int cmd_range_diff(int argc,
3351
OPT_INTEGER(0, "creation-factor",
3452
&range_diff_opts.creation_factor,
3553
N_("percentage by which creation is weighted")),
54+
OPT_CALLBACK(0, "max-memory", &range_diff_opts.max_memory,
55+
N_("size"),
56+
N_("maximum memory for cost matrix (default 4G)"),
57+
parse_max_memory),
3658
OPT_BOOL(0, "no-dual-color", &simple_color,
3759
N_("use simple diff colors")),
3860
OPT_PASSTHRU_ARGV(0, "notes", &other_arg,

log-tree.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,7 @@ static void show_diff_of_diff(struct rev_info *opt)
717717
struct range_diff_options range_diff_opts = {
718718
.creation_factor = opt->creation_factor,
719719
.dual_color = 1,
720+
.max_memory = RANGE_DIFF_MAX_MEMORY_DEFAULT,
720721
.diffopt = &opts
721722
};
722723

range-diff.c

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "apply.h"
2222
#include "revision.h"
2323

24+
2425
struct patch_util {
2526
/* For the search for an exact match */
2627
struct hashmap_entry e;
@@ -287,8 +288,8 @@ static void find_exact_matches(struct string_list *a, struct string_list *b)
287288
}
288289

289290
static int diffsize_consume(void *data,
290-
char *line UNUSED,
291-
unsigned long len UNUSED)
291+
char *line UNUSED,
292+
unsigned long len UNUSED)
292293
{
293294
(*(int *)data)++;
294295
return 0;
@@ -325,13 +326,24 @@ static int diffsize(const char *a, const char *b)
325326
}
326327

327328
static void get_correspondences(struct string_list *a, struct string_list *b,
328-
int creation_factor)
329+
int creation_factor, size_t max_memory)
329330
{
330331
int n = a->nr + b->nr;
331332
int *cost, c, *a2b, *b2a;
332333
int i, j;
333-
334-
ALLOC_ARRAY(cost, st_mult(n, n));
334+
size_t cost_size = st_mult(n, n);
335+
size_t cost_bytes = st_mult(sizeof(int), cost_size);
336+
if (cost_bytes >= max_memory) {
337+
struct strbuf cost_str = STRBUF_INIT;
338+
struct strbuf max_str = STRBUF_INIT;
339+
strbuf_humanise_bytes(&cost_str, cost_bytes);
340+
strbuf_humanise_bytes(&max_str, max_memory);
341+
die(_("range-diff: unable to compute the range-diff, since it "
342+
"exceeds the maximum memory for the cost matrix: %s "
343+
"(%"PRIuMAX" bytes) needed, %s (%"PRIuMAX" bytes) available"),
344+
cost_str.buf, (uintmax_t)cost_bytes, max_str.buf, (uintmax_t)max_memory);
345+
}
346+
ALLOC_ARRAY(cost, cost_size);
335347
ALLOC_ARRAY(a2b, n);
336348
ALLOC_ARRAY(b2a, n);
337349

@@ -351,7 +363,8 @@ static void get_correspondences(struct string_list *a, struct string_list *b,
351363
}
352364

353365
c = a_util->matching < 0 ?
354-
a_util->diffsize * creation_factor / 100 : COST_MAX;
366+
a_util->diffsize * creation_factor / 100 :
367+
COST_MAX;
355368
for (j = b->nr; j < n; j++)
356369
cost[i + n * j] = c;
357370
}
@@ -360,7 +373,8 @@ static void get_correspondences(struct string_list *a, struct string_list *b,
360373
struct patch_util *util = b->items[j].util;
361374

362375
c = util->matching < 0 ?
363-
util->diffsize * creation_factor / 100 : COST_MAX;
376+
util->diffsize * creation_factor / 100 :
377+
COST_MAX;
364378
for (i = a->nr; i < n; i++)
365379
cost[i + n * j] = c;
366380
}
@@ -539,7 +553,7 @@ static void output(struct string_list *a, struct string_list *b,
539553
if (i < a->nr && a_util->matching < 0) {
540554
if (!range_diff_opts->right_only)
541555
output_pair_header(&opts, patch_no_width,
542-
&buf, &dashes, a_util, NULL);
556+
&buf, &dashes, a_util, NULL);
543557
i++;
544558
continue;
545559
}
@@ -548,7 +562,7 @@ static void output(struct string_list *a, struct string_list *b,
548562
while (j < b->nr && b_util->matching < 0) {
549563
if (!range_diff_opts->left_only)
550564
output_pair_header(&opts, patch_no_width,
551-
&buf, &dashes, NULL, b_util);
565+
&buf, &dashes, NULL, b_util);
552566
b_util = ++j < b->nr ? b->items[j].util : NULL;
553567
}
554568

@@ -591,7 +605,8 @@ int show_range_diff(const char *range1, const char *range2,
591605
if (!res) {
592606
find_exact_matches(&branch1, &branch2);
593607
get_correspondences(&branch1, &branch2,
594-
range_diff_opts->creation_factor);
608+
range_diff_opts->creation_factor,
609+
range_diff_opts->max_memory);
595610
output(&branch1, &branch2, range_diff_opts);
596611
}
597612

range-diff.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
#include "strvec.h"
66

77
#define RANGE_DIFF_CREATION_FACTOR_DEFAULT 60
8+
#define RANGE_DIFF_MAX_MEMORY_DEFAULT \
9+
(sizeof(void*) >= 8 ? \
10+
((size_t)(1024L * 1024L) * (size_t)(4 * 1024L)) : /* 4GB on 64-bit */ \
11+
((size_t)(1024L * 1024L) * (size_t)(2 * 1024L))) /* 2GB on 32-bit */
812

913
/*
1014
* A much higher value than the default, when we KNOW we are comparing
@@ -17,6 +21,7 @@ struct range_diff_options {
1721
unsigned dual_color:1;
1822
unsigned left_only:1, right_only:1;
1923
unsigned include_merges:1;
24+
size_t max_memory;
2025
const struct diff_options *diffopt; /* may be NULL */
2126
const struct strvec *other_arg; /* may be NULL */
2227
};

0 commit comments

Comments
 (0)