Re: [PATCH] extend svn_subst_translate_string() to record whether re-encoding and/or line ending translation were performed (v. 2)

> I hope you aren't discouraged from working on the patch.

Nah :)

> To the point, I originally asked if your changes affected the performance
> of checkout/export.  That is not a reason to stop the patch in its tracks;
> it's a question that should be answered (either way) and the patch then
> proceed.  So, firstly, do your changes have any noticeable performance
> effect, or is the effect of the added per-line condition simply not
> noticeable?

I don't have benchmarks, but it does not seem that the performance of
my tests are noticeably degraded. These tests aren't extensive,
however, so I am not sure whether a much larger Subversion operation
that uses the changes (such as running the patched `svnsync` to sync
the GNU Nano repository) is impacted.

Note that when EOL_TRANSLATED is NULL, the overhead is a single NULL
check for each line.

> If the latter, then I apologize (to Daniel) for your having spent time
> writing patches (in various "creative" ways :)) that address what is
> a non-problem.

It's not a big deal, and I think that it helped me to understand the
code more fully. Besides, I may have another option.

This other idea is based on knowledge that b->repair is usually FALSE.
 Given this, I examined one of the if statements:

if ((! b->repair) && DIFFERENT_EOL_STRINGS(src_format,
*src_format_len, newline_buf, newline_len))

The `DIFFERENT_EOL_STRINGS` check will run whenever b->repair is
FALSE. So, I check `DIFFERENT_EOL_STRINGS` first and if that check is
TRUE, I then check !b->repair, else check to see whether
*b->eol_translated needs to be set to TRUE:
<https://github.com/dtrebbien/subversion/commit/d22329a54dcf58cddc2b618f913597c6defbcb2d>.
 What do you all think?

The upside is that unnecessary NULL checks of TRANSLATED_EOL are
dynamically elided under normal conditions.

The downsides are:

1.) Another assumption must be made (namely, that the EOL_STR string
is not varied between calls to translate_newline() using the same
translation baton).

2.) It complicates the logic.

3.) This penalizes repair translations.

> > p.s. To avoid confusion, you can call me Danny if you would like.  I
> > like that name, and it comes in handy when working with other Daniels
> > :)
>
> And I'm danielsh, if it helps.
>
> (btw, the firstnames distribution in COMMITTERS is... interesting.)

You piqued my curiosity. Here is a frequency table of first names of
the active full committers:

5|Daniel
3|David
3|Stefan
2|Ben
2|Greg
2|Mark
1|Arfrever
1|Bert
1|Blair
1|Branko
1|Brian
1|C
1|Erik
1|Garrett
1|Hyrum
1|Ivan
1|Jani
1|Jeremy
1|Jim
1|Joe
1|John
1|Julian
1|Justin
1|Kamesh
1|Karl
1|Kouhei
1|Lieven
1|Max
1|Neels
1|Paul
1|Peter
1|Philip
1|Sander
1|Senthil
1|Stephen
1|Vlad

That's a lot of Daniels.
Received on 2010-11-27 18:47:12 CET