"make check" times

"make check" times

[DJ Delorie]

While digging for some low-hanging fruit in "make" times, I did this:

diff --git a/scripts/merge-test-results.sh b/scripts/merge-test-results.sh
index 919bbae253..7088ef6996 100755
--- a/scripts/merge-test-results.sh
+++ b/scripts/merge-test-results.sh
@@ -35,7 +35,11 @@ case $type in
     subdir=${subdir:+$subdir/}
     for t in "$@"; do
       if [ -s "$objpfx$t.test-result" ]; then
-	head -n1 "$objpfx$t.test-result"
+	  #head -n1 "$objpfx$t.test-result"
+	  exec 6<"$objpfx$t.test-result"
+	  read line <&6
+	  echo $line
+	  exec 6<&-
       else
 	echo "UNRESOLVED: $subdir$t"
       fi

That one instance of "head" is called over 6000 times per "make
check", and as it's the only non-builtin in that script, it adds about
11 seconds of overhead compared to just reading that one line with
builtins.

My question here is: how much of a time savings is worth the
complexity of said savings?

Re: "make check" times

[Carlos O'Donell]

On 7/19/19 6:11 PM, DJ Delorie wrote:
> While digging for some low-hanging fruit in "make" times, I did this:
> 
> diff --git a/scripts/merge-test-results.sh b/scripts/merge-test-results.sh
> index 919bbae253..7088ef6996 100755
> --- a/scripts/merge-test-results.sh
> +++ b/scripts/merge-test-results.sh
> @@ -35,7 +35,11 @@ case $type in
>       subdir=${subdir:+$subdir/}
>       for t in "$@"; do
>         if [ -s "$objpfx$t.test-result" ]; then
> -	head -n1 "$objpfx$t.test-result"
> +	  #head -n1 "$objpfx$t.test-result"
> +	  exec 6<"$objpfx$t.test-result"
> +	  read line <&6
> +	  echo $line
> +	  exec 6<&-
>         else
>   	echo "UNRESOLVED: $subdir$t"
>         fi
> 
> That one instance of "head" is called over 6000 times per "make
> check", and as it's the only non-builtin in that script, it adds about
> 11 seconds of overhead compared to just reading that one line with
> builtins.
> 
> My question here is: how much of a time savings is worth the
> complexity of said savings?

One needs to tackle the problem the other way around.

* First determine those things we cannot change and remove them
   form the accounting.
   - We cannot avoid compiling each file.

* Whatever is left over after removing the things we can't
   avoid doing is our "overhead".

* Sort the "overhead" by the amount of time spent, and find ways
   to reduce it.

If the "overhead" is small, then there isn't much value in adding
complexity.

If the "overhead" is large, we may want to rewrite the whole build
system to get that time back.

Does that make sense?

I can't answer your question without knowing how much is 11 seconds
out of the total time taken.

If the build took 20 seconds to complete, then yes this is a valuable
change ;-)

-- 
Cheers,
Carlos.

Re: "make check" times

[DJ Delorie]

"Carlos O'Donell" <carlos@redhat.com> writes:
> I can't answer your question without knowing how much is 11 seconds
> out of the total time taken.

In this case, it's about a third of "make check" if you don't need to
rebuild/recheck anything (reduces 30 seconds to 19 seconds), but
compared to a full test run (lots of minutes) it's in the noise.

I've been looking at it from a "why does running one test take so long"
viewpoint - when a test takes under a second, but "make check" takes 30
seconds, there's a lot of overhead.

>   - We cannot avoid compiling each file.

There are ways of optimizing this, too, though... like better
parallelism.

Re: "make check" times

[Carlos O'Donell]

On 7/19/19 9:31 PM, DJ Delorie wrote:
> "Carlos O'Donell" <carlos@redhat.com> writes:
>> I can't answer your question without knowing how much is 11 seconds
>> out of the total time taken.
> 
> In this case, it's about a third of "make check" if you don't need to
> rebuild/recheck anything (reduces 30 seconds to 19 seconds), but
> compared to a full test run (lots of minutes) it's in the noise.

Then I think this is a worthwhile fix.

In the old days I often would remove *one* output file to trigger the
test to be re-run and then just hit 'make check'.

This fix would, IIUC, accelerate that use case, which is probably a
common "newcomer" behaviour e.g. remove the test target result, and
rerun 'make check' to re-run that test.
  
> I've been looking at it from a "why does running one test take so long"
> viewpoint - when a test takes under a second, but "make check" takes 30
> seconds, there's a lot of overhead.

Agreed. It should be way way less.
  
>>    - We cannot avoid compiling each file.
> 
> There are ways of optimizing this, too, though... like better
> parallelism.

Agreed completely. To do that we need to start understanding our
dependency graph, and I know you know that ;-)

-- 
Cheers,
Carlos.

Re: "make check" times

[DJ Delorie]

Pinging this ancient patch, now that I've finished the build times
report which answers the questions asked about this:

Original patch thread:
https://sourceware.org/pipermail/libc-alpha/2019-July/105187.html
Build times report thread:
https://sourceware.org/pipermail/libc-alpha/2020-March/111782.html

DJ Delorie <dj@redhat.com> writes:
> While digging for some low-hanging fruit in "make" times, I did this:
>
> diff --git a/scripts/merge-test-results.sh b/scripts/merge-test-results.sh
> index 919bbae253..7088ef6996 100755
> --- a/scripts/merge-test-results.sh
> +++ b/scripts/merge-test-results.sh
> @@ -35,7 +35,11 @@ case $type in
>      subdir=${subdir:+$subdir/}
>      for t in "$@"; do
>        if [ -s "$objpfx$t.test-result" ]; then
> -	head -n1 "$objpfx$t.test-result"
> +	  #head -n1 "$objpfx$t.test-result"
> +	  exec 6<"$objpfx$t.test-result"
> +	  read line <&6
> +	  echo $line
> +	  exec 6<&-
>        else
>  	echo "UNRESOLVED: $subdir$t"
>        fi
>
> That one instance of "head" is called over 6000 times per "make
> check", and as it's the only non-builtin in that script, it adds about
> 11 seconds of overhead compared to just reading that one line with
> builtins.
>
> My question here is: how much of a time savings is worth the
> complexity of said savings?

Re: "make check" times

[Carlos O'Donell]

On 3/11/20 3:32 PM, DJ Delorie via Libc-alpha wrote:
> 
> Pinging this ancient patch, now that I've finished the build times
> report which answers the questions asked about this:
> 
> Original patch thread:
> https://sourceware.org/pipermail/libc-alpha/2019-July/105187.html
> Build times report thread:
> https://sourceware.org/pipermail/libc-alpha/2020-March/111782.html
> 
> DJ Delorie <dj@redhat.com> writes:
>> While digging for some low-hanging fruit in "make" times, I did this:
>>
>> diff --git a/scripts/merge-test-results.sh b/scripts/merge-test-results.sh
>> index 919bbae253..7088ef6996 100755
>> --- a/scripts/merge-test-results.sh
>> +++ b/scripts/merge-test-results.sh
>> @@ -35,7 +35,11 @@ case $type in
>>      subdir=${subdir:+$subdir/}
>>      for t in "$@"; do
>>        if [ -s "$objpfx$t.test-result" ]; then
>> -	head -n1 "$objpfx$t.test-result"
>> +	  #head -n1 "$objpfx$t.test-result"
>> +	  exec 6<"$objpfx$t.test-result"
>> +	  read line <&6
>> +	  echo $line
>> +	  exec 6<&-
>>        else
>>  	echo "UNRESOLVED: $subdir$t"
>>        fi
>>
>> That one instance of "head" is called over 6000 times per "make
>> check", and as it's the only non-builtin in that script, it adds about
>> 11 seconds of overhead compared to just reading that one line with
>> builtins.
>>
>> My question here is: how much of a time savings is worth the
>> complexity of said savings?
> 

This is worth it.

OK for master.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

-- 
Cheers,
Carlos.

Re: "make check" times

[Andreas Schwab]

On Mär 11 2020, DJ Delorie via Libc-alpha wrote:

>> diff --git a/scripts/merge-test-results.sh b/scripts/merge-test-results.sh
>> index 919bbae253..7088ef6996 100755
>> --- a/scripts/merge-test-results.sh
>> +++ b/scripts/merge-test-results.sh
>> @@ -35,7 +35,11 @@ case $type in
>>      subdir=${subdir:+$subdir/}
>>      for t in "$@"; do
>>        if [ -s "$objpfx$t.test-result" ]; then
>> -	head -n1 "$objpfx$t.test-result"
>> +	  #head -n1 "$objpfx$t.test-result"
>> +	  exec 6<"$objpfx$t.test-result"
>> +	  read line <&6
>> +	  echo $line
>> +	  exec 6<&-

Why do you need to use the exec dance?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: "make check" times

[DJ Delorie]

Andreas Schwab <schwab@linux-m68k.org> writes:
> Why do you need to use the exec dance?

Is "read line <$file" portable posix shell?

Re: "make check" times

[Joseph Myers]

On Tue, 31 Mar 2020, Carlos O'Donell via Libc-alpha wrote:

> >> That one instance of "head" is called over 6000 times per "make
> >> check", and as it's the only non-builtin in that script, it adds about
> >> 11 seconds of overhead compared to just reading that one line with
> >> builtins.
> >>
> >> My question here is: how much of a time savings is worth the
> >> complexity of said savings?
> > 
> 
> This is worth it.
> 
> OK for master.

It definitely needs a comment in the script explaining that it's 
deliberately using only built-in shell functionality for speed.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: "make check" times

[Andreas Schwab]

On Apr 01 2020, DJ Delorie wrote:

> Andreas Schwab <schwab@linux-m68k.org> writes:
>> Why do you need to use the exec dance?
>
> Is "read line <$file" portable posix shell?

Sure, why not?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: "make check" times

[DJ Delorie]

Andreas Schwab <schwab@linux-m68k.org> writes:
>> Is "read line <$file" portable posix shell?
>
> Sure, why not?

Well, when you put it like that...

Yeah, tested on solaris 9's various shells, works fine.

Re: "make check" times

[Carlos O'Donell]

On 4/1/20 1:10 PM, Joseph Myers wrote:
> On Tue, 31 Mar 2020, Carlos O'Donell via Libc-alpha wrote:
> 
>>>> That one instance of "head" is called over 6000 times per "make
>>>> check", and as it's the only non-builtin in that script, it adds about
>>>> 11 seconds of overhead compared to just reading that one line with
>>>> builtins.
>>>>
>>>> My question here is: how much of a time savings is worth the
>>>> complexity of said savings?
>>>
>>
>> This is worth it.
>>
>> OK for master.
> 
> It definitely needs a comment in the script explaining that it's 
> deliberately using only built-in shell functionality for speed.

Good point. Agreed.

-- 
Cheers,
Carlos.

Re: "make check" times

[DJ Delorie]

Carlos writes:
> This is worth it.

Thanks - reviewed-by removed as the patch has changed.

Andreas writes:
> Why do you need to use the exec dance?

Uneeded, fixed.

Joseph writes:
> It definitely needs a comment in the script

Added.

Ok now?

From 7af6c4b85394fd6cd2988e73331d6a0bc4ee4882 Mon Sep 17 00:00:00 2001
From: DJ Delorie <dj@redhat.com>
Date: Wed, 1 Apr 2020 15:33:00 -0400
Subject: Optimize scripts/merge-test-results.sh

The inner loop is called thousands of times per "make check" even
if there's otherwise nothing to do.  Avoid calling /bin/head all
those times when a builtin will do.

diff --git a/scripts/merge-test-results.sh b/scripts/merge-test-results.sh
index 573a44d8cf..6fd0a28dc8 100755
--- a/scripts/merge-test-results.sh
+++ b/scripts/merge-test-results.sh
@@ -35,7 +35,11 @@ case $type in
     subdir=${subdir:+$subdir/}
     for t in "$@"; do
       if [ -s "$objpfx$t.test-result" ]; then
-	head -n1 "$objpfx$t.test-result"
+	# This loop is called thousands of times even when there's
+	# nothing to do.  Avoid using non-built-in commands (like
+	# /bin/head) where possible.
+	read line <"$objpfx$t.test-result"
+	echo $line
       else
 	echo "UNRESOLVED: $subdir$t"
       fi

Re: "make check" times

[Andreas Schwab]

On Apr 01 2020, DJ Delorie via Libc-alpha wrote:

> diff --git a/scripts/merge-test-results.sh b/scripts/merge-test-results.sh
> index 573a44d8cf..6fd0a28dc8 100755
> --- a/scripts/merge-test-results.sh
> +++ b/scripts/merge-test-results.sh
> @@ -35,7 +35,11 @@ case $type in
>      subdir=${subdir:+$subdir/}
>      for t in "$@"; do
>        if [ -s "$objpfx$t.test-result" ]; then
> -	head -n1 "$objpfx$t.test-result"
> +	# This loop is called thousands of times even when there's
> +	# nothing to do.  Avoid using non-built-in commands (like
> +	# /bin/head) where possible.
> +	read line <"$objpfx$t.test-result"
> +	echo $line

Please use proper quoting.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: "make check" times

[DJ Delorie]

Andreas Schwab <schwab@linux-m68k.org> writes:
>> -	head -n1 "$objpfx$t.test-result"
>> +	# This loop is called thousands of times even when there's
>> +	# nothing to do.  Avoid using non-built-in commands (like
>> +	# /bin/head) where possible.
>> +	read line <"$objpfx$t.test-result"
>> +	echo $line
>
> Please use proper quoting.

Could you be more specific?

Re: "make check" times

[Andreas Schwab]

Never expand a variable unquoted.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: "make check" times

[DJ Delorie]

Andreas Schwab <schwab@linux-m68k.org> writes:
> Never expand a variable unquoted.

How's this?  Also, comment updated to note we assume "echo" is a
builtin.

From 98e461fd50f1617acc78caeaa70547b92a54cf5c Mon Sep 17 00:00:00 2001
From: DJ Delorie <dj@redhat.com>
Date: Wed, 1 Apr 2020 15:33:00 -0400
Subject: Optimize scripts/merge-test-results.sh

The inner loop is called thousands of times per "make check" even
if there's otherwise nothing to do.  Avoid calling /bin/head all
those times when a builtin will do.

diff --git a/scripts/merge-test-results.sh b/scripts/merge-test-results.sh
index 573a44d8cf..e75123a730 100755
--- a/scripts/merge-test-results.sh
+++ b/scripts/merge-test-results.sh
@@ -35,7 +35,12 @@ case $type in
     subdir=${subdir:+$subdir/}
     for t in "$@"; do
       if [ -s "$objpfx$t.test-result" ]; then
-	head -n1 "$objpfx$t.test-result"
+	# This loop is called thousands of times even when there's
+	# nothing to do.  Avoid using non-built-in commands (like
+	# /bin/head) where possible.  We assume "echo" is typically a
+	# built-in.
+	read line < "$objpfx$t.test-result"
+	echo "$line"
       else
 	echo "UNRESOLVED: $subdir$t"
       fi

Re: "make check" times

[Carlos O'Donell]

On 4/1/20 3:58 PM, DJ Delorie via Libc-alpha wrote:
> 
> Andreas Schwab <schwab@linux-m68k.org> writes:
>> Never expand a variable unquoted.
> 
> How's this?  Also, comment updated to note we assume "echo" is a
> builtin.
> 

LGTM. All variable expansions quoted. Non-builtins removed for performance.
Comment added to explain what we're doing.

Even if you could run `type -t echo` to determine if echo was a builitin,
I'm not sure we could do anything different with that knowledge. It's probably
still faster to do read+echo than head.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> From 98e461fd50f1617acc78caeaa70547b92a54cf5c Mon Sep 17 00:00:00 2001
> From: DJ Delorie <dj@redhat.com>
> Date: Wed, 1 Apr 2020 15:33:00 -0400
> Subject: Optimize scripts/merge-test-results.sh
> 
> The inner loop is called thousands of times per "make check" even
> if there's otherwise nothing to do.  Avoid calling /bin/head all
> those times when a builtin will do.
> 
> diff --git a/scripts/merge-test-results.sh b/scripts/merge-test-results.sh
> index 573a44d8cf..e75123a730 100755
> --- a/scripts/merge-test-results.sh
> +++ b/scripts/merge-test-results.sh
> @@ -35,7 +35,12 @@ case $type in
>      subdir=${subdir:+$subdir/}
>      for t in "$@"; do
>        if [ -s "$objpfx$t.test-result" ]; then
> -	head -n1 "$objpfx$t.test-result"
> +	# This loop is called thousands of times even when there's
> +	# nothing to do.  Avoid using non-built-in commands (like
> +	# /bin/head) where possible.  We assume "echo" is typically a
> +	# built-in.
> +	read line < "$objpfx$t.test-result"
> +	echo "$line"
>        else
>  	echo "UNRESOLVED: $subdir$t"
>        fi
> 


-- 
Cheers,
Carlos.

Re: "make check" times

[Joseph Myers]

On Wed, 1 Apr 2020, Carlos O'Donell via Libc-alpha wrote:

> On 4/1/20 3:58 PM, DJ Delorie via Libc-alpha wrote:
> > 
> > Andreas Schwab <schwab@linux-m68k.org> writes:
> >> Never expand a variable unquoted.
> > 
> > How's this?  Also, comment updated to note we assume "echo" is a
> > builtin.
> > 
> 
> LGTM. All variable expansions quoted. Non-builtins removed for performance.
> Comment added to explain what we're doing.

Strictly speaking a safe "read" would be IFS= read -r line (IFS= to avoid 
stripping leading whitespace, -r to avoid backslash escapes being 
processed).  However, in this case that's not needed because valid test 
result lines do not start with whitespace and do not contain backslash 
(and nor do they start -n or -e, being cases where the "echo" would be 
unsafe).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: "make check" times

[DJ Delorie]

Joseph Myers <joseph@codesourcery.com> writes:
> Strictly speaking a safe "read" would be IFS= read -r line (IFS= to avoid 
> stripping leading whitespace, -r to avoid backslash escapes being 
> processed).  However, in this case that's not needed because valid test 
> result lines do not start with whitespace and do not contain backslash 
> (and nor do they start -n or -e, being cases where the "echo" would be 
> unsafe).

I did think of those, but yeah, we're parsing our own data and it's
"safe".

Re: "make check" times

[Matt Turner]

On Wed, Apr 1, 2020 at 12:42 PM Andreas Schwab <schwab@linux-m68k.org> wrote:
>
> On Apr 01 2020, DJ Delorie via Libc-alpha wrote:
>
> > diff --git a/scripts/merge-test-results.sh b/scripts/merge-test-results.sh
> > index 573a44d8cf..6fd0a28dc8 100755
> > --- a/scripts/merge-test-results.sh
> > +++ b/scripts/merge-test-results.sh
> > @@ -35,7 +35,11 @@ case $type in
> >      subdir=${subdir:+$subdir/}
> >      for t in "$@"; do
> >        if [ -s "$objpfx$t.test-result" ]; then
> > -     head -n1 "$objpfx$t.test-result"
> > +     # This loop is called thousands of times even when there's
> > +     # nothing to do.  Avoid using non-built-in commands (like
> > +     # /bin/head) where possible.
> > +     read line <"$objpfx$t.test-result"
> > +     echo $line
>
> Please use proper quoting.

I see a lot of replies from you (like this) that read as very aloof.
Just from a pragmatic perspective, explaining what you mean initially
would save you the inevitable round-trip.

Re: "make check" times

[DJ Delorie]

Sorry for the delay.  I applied Joseph's suggestions, retested, and
committed the patch as attached.

From 43be1301f65f62b48ed6d200657d47c79ee22bc3 Mon Sep 17 00:00:00 2001
From: DJ Delorie <dj@redhat.com>
Date: Wed, 1 Apr 2020 15:33:00 -0400
Subject: Optimize scripts/merge-test-results.sh

The inner loop is called thousands of times per "make check" even
if there's otherwise nothing to do.  Avoid calling /bin/head all
those times when a builtin will do.

diff --git a/scripts/merge-test-results.sh b/scripts/merge-test-results.sh
index 573a44d8cf..1e236db4a7 100755
--- a/scripts/merge-test-results.sh
+++ b/scripts/merge-test-results.sh
@@ -35,7 +35,12 @@ case $type in
     subdir=${subdir:+$subdir/}
     for t in "$@"; do
       if [ -s "$objpfx$t.test-result" ]; then
-	head -n1 "$objpfx$t.test-result"
+	# This loop is called thousands of times even when there's
+	# nothing to do.  Avoid using non-built-in commands (like
+	# /bin/head) where possible.  We assume "echo" is typically a
+	# built-in.
+	IFS= read -r line < "$objpfx$t.test-result"
+	echo "$line"
       else
 	echo "UNRESOLVED: $subdir$t"
       fi

Re: "make check" times

[Andreas Schwab]

On Okt 05 2020, DJ Delorie wrote:

> diff --git a/scripts/merge-test-results.sh b/scripts/merge-test-results.sh
> index 573a44d8cf..1e236db4a7 100755
> --- a/scripts/merge-test-results.sh
> +++ b/scripts/merge-test-results.sh
> @@ -35,7 +35,12 @@ case $type in
>      subdir=${subdir:+$subdir/}
>      for t in "$@"; do
>        if [ -s "$objpfx$t.test-result" ]; then
> -	head -n1 "$objpfx$t.test-result"
> +	# This loop is called thousands of times even when there's
> +	# nothing to do.  Avoid using non-built-in commands (like
> +	# /bin/head) where possible.  We assume "echo" is typically a
> +	# built-in.
> +	IFS= read -r line < "$objpfx$t.test-result"
> +	echo "$line"

If you want to avoid any interpretation you should use printf %s.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: "make check" times

[DJ Delorie]

Is printf a guaranteed builtin in posix shells?

(and as noted back when, the data we're reading does not require such
protections anyway)

Re: "make check" times

[Paul Eggert]

On 10/5/20 2:51 PM, DJ Delorie via Libc-alpha wrote:
> Is printf a guaranteed builtin in posix shells?

It's not a "Special Built-In Utility" in the POSIX sense, but neither are 'echo' 
or 'read'.

To answer what I think is your real question, these days I believe that "printf 
'%s\n' 'FOO'" is typically nearly as fast as "echo 'FOO'" on the shells that 
glibc developers use. I just tried a simple benchmark with bash and dash on 
Ubuntu 20.04 x86-64 with a loop that just output 'x' on a line, and the echo 
version was about 20% faster with dash and about 15% faster with Bash.

Re: "make check" times

[DJ Delorie]

Paul Eggert <eggert@cs.ucla.edu> writes:
> To answer what I think is your real question, these days I believe that "printf 
> '%s\n' 'FOO'" is typically nearly as fast as "echo 'FOO'" on the shells that 
> glibc developers use. I just tried a simple benchmark with bash and dash on 
> Ubuntu 20.04 x86-64 with a loop that just output 'x' on a line, and the echo 
> version was about 20% faster with dash and about 15% faster with Bash.

Oooo, that.  Faster is better :-)

Re: "make check" times

[Andreas Schwab]

On Okt 05 2020, DJ Delorie wrote:

> (and as noted back when, the data we're reading does not require such
> protections anyway)

Neither is read -r needed.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: "make check" times

[DJ Delorie]

Andreas Schwab <schwab@linux-m68k.org> writes:
>> (and as noted back when, the data we're reading does not require such
>> protections anyway)
>
> Neither is read -r needed.

True, but I felt that not including that and IFS= meant I was putting a
bad example out there for others to follow.