[PATCH] move LCS table away from the stack

[hjemli at gmail.com (Lars Hjemli)]

On Sun, Sep 11, 2011 at 02:44, Jamie Couture <jamie.couture at gmail.com> wrote:
> +/*
> + * ssdiff line limits
> + */
> +#ifndef MAX_SSDIFF_M
> +#define MAX_SSDIFF_M 1024
> +#endif
> +
> +#ifndef MAX_SSDIFF_N
> +#define MAX_SSDIFF_N 1024
> +#endif
> +#define MAX_SSDIFF_SIZE ((MAX_SSDIFF_M) * (MAX_SSDIFF_N))

I think this limit should be more like 128*128 for a few reasons:
* ss-diff for lines longer than 128 chars probably isn't very useful
(you'd need a very wide monitor)
* cpu time spent in LCS() seems to be propotional to avg(linelength)^2


> ?static char *longest_common_subsequence(char *A, char *B)
> ?{
> ? ? ? ?int i, j, ri;
> ? ? ? ?int m = strlen(A);
> ? ? ? ?int n = strlen(B);
> - ? ? ? int L[m + 1][n + 1];
> - ? ? ? int tmp1, tmp2;
> + ? ? ? int **L;
> + ? ? ? int tmp1, tmp2, length;
> ? ? ? ?int lcs_length;
> ? ? ? ?char *result;
>
> + ? ? ? length = (m + 1) * (n + 1);
> +
> + ? ? ? // We bail if the lines are too long
> + ? ? ? if (length > MAX_SSDIFF_SIZE)
> + ? ? ? ? ? ? ? return NULL;
> +
> + ? ? ? L = create_lcs_table(m + 1, n + 1);
> +
> ? ? ? ?for (i = m; i >= 0; i--) {
> ? ? ? ? ? ? ? ?for (j = n; j >= 0; j--) {
> ? ? ? ? ? ? ? ? ? ? ? ?if (A[i] == '\0' || B[j] == '\0') {
> @@ -59,6 +84,9 @@ static char *longest_common_subsequence(char *A, char *B)
> ? ? ? ? ? ? ? ? ? ? ? ?j += 1;
> ? ? ? ? ? ? ? ?}
> ? ? ? ?}
> +
> + ? ? ? free(*L);
> + ? ? ? free(L);
> ? ? ? ?return result;
> ?}

This function is potentially invoked for each diff-line, right? If so,
why not prepare a "shared" lcs-table in the caller (expecting
worst-case linelength) to avoid the setup/teardown of the table for
each line?

--
larsh