diff mbox

[5/5] check_GNU_style.sh: Fix tab size in 80 characters check

Message ID 55599DAD.6030800@mentor.com
State New
Headers show

Commit Message

Tom de Vries May 18, 2015, 8:07 a.m. UTC 
On 12-05-15 17:16, Jeff Law wrote:
>> [PATCH 5/5] check_GNU_style.sh: Fix tab size in 80 characters check
>>
>> 2015-05-11  Tom de Vries<tom@codesourcery.com>
>>
>>     * check_GNU_style.sh (col): Fix tab size.
> OK.

Hi Jeff,

I.

I noticed a performance degradation due to this patch:
...
$ cat gcc/tree-ssa-tail-merge.c | awk '{printf "+%s\n", $0}' | time -p 
./contrib/check_GNU_style.sh -
   ...
real 4.10
user 0.71
sys 6.77
...

Before this patch, the 'real' time was roughly a factor 80 smaller:
...
real 0.05
user 0.02
sys 0.03
...

This degradation is due to the fact that the patch does the 80 chars check 
line-by-line, and invokes processes for each new line.


II.

Attached follow-up patch rewrites the 80 chars check to handle a file at a time 
rather than a line at a time, and gets performance back to normal:
...
real 0.07
user 0.03
sys 0.05
...

As a bonus, the bit longer than 80 chars is now printed in red, similar to how 
the other checks show the output.

OK for trunk?

Thanks,
- Tom

Comments

Jeff Law May 18, 2015, 3:40 p.m. UTC | #1 
On 05/18/2015 02:07 AM, Tom de Vries wrote:
> On 12-05-15 17:16, Jeff Law wrote:
>>> [PATCH 5/5] check_GNU_style.sh: Fix tab size in 80 characters check
>>>
>>> 2015-05-11  Tom de Vries<tom@codesourcery.com>
>>>
>>>     * check_GNU_style.sh (col): Fix tab size.
>> OK.
>
> Hi Jeff,
>
> I.
>
> I noticed a performance degradation due to this patch:
> ...
> $ cat gcc/tree-ssa-tail-merge.c | awk '{printf "+%s\n", $0}' | time -p
> ./contrib/check_GNU_style.sh -
>    ...
> real 4.10
> user 0.71
> sys 6.77
> ...
>
> Before this patch, the 'real' time was roughly a factor 80 smaller:
> ...
> real 0.05
> user 0.02
> sys 0.03
> ...
>
> This degradation is due to the fact that the patch does the 80 chars
> check line-by-line, and invokes processes for each new line.
>
>
> II.
>
> Attached follow-up patch rewrites the 80 chars check to handle a file at
> a time rather than a line at a time, and gets performance back to normal:
> ...
> real 0.07
> user 0.03
> sys 0.05
> ...
>
> As a bonus, the bit longer than 80 chars is now printed in red, similar
> to how the other checks show the output.
>
> OK for trunk?
Yes, this is fine.

jeff
diff mbox

Patch

check_GNU_style.sh: Don't do 80 char check line by line

2015-05-18  Tom de Vries  <tom@codesourcery.com>

	* check_GNU_style.sh: Add temp files tmp2 and tmp3.
	(cat_with_prefix): New function, using global variable prefix.
	(col): Make prefix a global variable. Rewrite to process file at a time
	rather than line at a time.  Print part longer than 80 chars in red.
---
 contrib/check_GNU_style.sh | 70 ++++++++++++++++++++++++++++++++++------------
 1 file changed, 52 insertions(+), 18 deletions(-)

diff --git a/contrib/check_GNU_style.sh b/contrib/check_GNU_style.sh
index ab59b1e..033a2c9 100755
--- a/contrib/check_GNU_style.sh
+++ b/contrib/check_GNU_style.sh
@@ -65,10 +65,12 @@  fi
 
 inp=check_GNU_style.inp
 tmp=check_GNU_style.tmp
+tmp2=check_GNU_style.2.tmp
+tmp3=check_GNU_style.3.tmp
 
 # Remove $tmp on exit and various signals.
-trap "rm -f $inp $tmp $stdin_tmp" 0
-trap "rm -f $inp $tmp $stdin_tmp; exit 1" 1 2 3 5 9 13 15
+trap "rm -f $inp $tmp $tmp2 $tmp3 $stdin_tmp" 0
+trap "rm -f $inp $tmp $tmp2 $tmp3 $stdin_tmp; exit 1" 1 2 3 5 9 13 15
 
 if [ $nfiles -eq 1 ]; then
     # There's no need for the file prefix if we're dealing only with one file.
@@ -80,6 +82,17 @@  grep $format '^+' $files \
     | grep -v ':+++' \
     > $inp
 
+cat_with_prefix ()
+{
+    local f="$1"
+
+    if [ "$prefix" = "" ]; then
+	cat "$f"
+    else
+	awk "{printf "%s%s\n", $prefix, \$0}" $f
+    fi
+}
+
 # Grep
 g (){
     local msg="$1"
@@ -134,10 +147,11 @@  vg (){
 
 col (){
     local msg="$1"
+
     local first=true
     local f
     for f in $files; do
-	local prefix=""
+	prefix=""
 	if [ $nfiles -ne 1 ]; then
 	    prefix="$f:"
 	fi
@@ -148,22 +162,42 @@  col (){
 	    | grep -v ':+++' \
 	    > $tmp
 
-	cat $tmp | while IFS= read -r line; do
-	    local longline
-	    # Filter out the line number prefix and the patch line modifier '+'
-	    # to obtain the bare line, before we use expand.
-	    longline=$(echo "$line" \
-		| sed 's/^[0-9]*:+//' \
-		| expand \
-		| awk '{ if (length($0) > 80) print $0}')
-	    if [ "$longline" != "" ]; then
-		if $first; then
-		    printf "\n$msg\n"
-		    first=false
-		fi
-		echo "$prefix$line"
+	# Keep only line number prefix and patch modifier '+'.
+	cat "$tmp" \
+	    | sed 's/\(^[0-9][0-9]*:+\).*/\1/' \
+	    > "$tmp2"
+
+	# Remove line number prefix and patch modifier '+'.
+	# Expand tabs to spaces according to tab positions.
+	# Keep long lines, make short lines empty.  Print the part past 80 chars
+	# in red.
+	cat "$tmp" \
+	    | sed 's/^[0-9]*:+//' \
+	    | expand \
+	    | awk '{ \
+		     if (length($0) > 80) \
+		       printf "%s\033[1;31m%s\033[0m\n", \
+			      substr($0,1,80), \
+			      substr($0,81); \
+		     else \
+		       print "" \
+		   }' \
+	    > "$tmp3"
+
+	# Combine prefix back with long lines.
+	# Filter out empty lines.
+	local found=false
+	paste -d '' "$tmp2" "$tmp3" \
+	    | grep -v '^[0-9][0-9]*:+$' \
+	    > "$tmp" && found=true
+
+	if $found; then
+	    if $first; then
+		printf "\n$msg\n"
+		first=false
 	    fi
-	done
+	    cat_with_prefix "$tmp"
+	fi
     done
 }
 
-- 
1.9.1