You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2021/07/08 12:42:58 UTC

Re: [PATCH] limit diff effort to fix performance issue

On Tue, Jun 08, 2021 at 05:58:14PM +0200, Johan Corveleyn wrote:
> Also: one of the last things that Morten Kloster committed in 2011 was
> another performance improvement, namely "restarting the LCS" in some
> specific cases. At the time, I didn't have enough time to review this
> properly, and had some reservations (but mainly lack of time), so I
> asked him to commit it on a branch (diff-improvements). This feature
> branch never made it to trunk. Just as another "shot", perhaps you
> could try r1140247 and see if it helps?

Unfortunately, r1140247 doesn't help in my case.
I see the new logic which was added trigger once very early on.
But it soon gets stuck in the loop forever just like the code on trunk does.

The code added in r1140247 seems unfinished and untested.
It has a bug in clear_fp() which clang warns about (see below).
Fixing that alone doesn't help my test case though.

subversion/libsvn_diff/lcs.c:264:19: warning: incompatible pointer types assigning to 'svn_diff__lcs_t *'
      (aka 'struct svn_diff__lcs_t *') from 'svn_diff__lcs_t **' (aka 'struct svn_diff__lcs_t **'); dereference with *
      [-Wincompatible-pointer-types]
        lcs->next = lcs_freelist;
                  ^ ~~~~~~~~~~~~
                    *
subversion/libsvn_diff/lcs.c:265:22: warning: incompatible pointer types assigning to 'svn_diff__lcs_t **'
      (aka 'struct svn_diff__lcs_t **') from 'svn_diff__lcs_t *' (aka 'struct svn_diff__lcs_t *'); take the address with &
      [-Wincompatible-pointer-types]
        lcs_freelist = lcs;
                     ^ ~~~
                       &
2 warnings generated.