On Fri, Apr 03, 2015 at 11:19:24AM +0900, Yi, EungJun wrote:
> > I timed this one versus the existing diff-highlight. It's about 7%
> > slower. That's not great, but is acceptable to me. The String::Multibyte
> > version was a lot faster, which was nice (but I'm still unclear on
> > _why_).
>
> I think the reason is here:
>
> > sub split_line {
> > local $_ = shift;
> > return map { /$COLOR/ ? $_ : ($mbcs ? $mbcs->strsplit('', $_) : split
> > //) }
> > split /($COLOR)/;
> > }
>
> I removed "*" from "split /($COLOR*)/". Actually I don't know why "*"
> was required but I need to remove it to make my patch works correctly.
Ah, OK, that makes more sense. The "*" was meant to handle the case of
multiple groups of ANSI colors in a row. But I think it should have been
"+" in that case, as we would otherwise split on the empty field, which
would mean character-by-character. And the second "split" in the map
would then be superfluous, which would break your patch (we've already
split the multi-byte characters before we even hit $mbcs->strsplit).
Kyle's patch does not care, because it tweaks the string so that normal
split works. Which means there is an easy speedup here. :)
Doing:
diff --git a/contrib/diff-highlight/diff-highlight
b/contrib/diff-highlight/diff-highlight
index 08c88bb..1c4b599 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -165,7 +165,7 @@ sub highlight_pair {
sub split_line {
local $_ = shift;
return map { /$COLOR/ ? $_ : (split //) }
- split /($COLOR*)/;
+ split /($COLOR+)/;
}
sub highlight_line {
gives me a 25% speed improvement, and the same output processing
git.git's entire "git log -p" output.
I thought that meant we could also optimize out the "map" call entirely,
and just use the first split (with "*") to end up with a list of $COLOR
chunks and single characters, but it does not seem to work. So maybe I
am misreading something about what is going on.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
