For the record ...

On 21/02/2020 22:31, Kyle Evans wrote:
On Fri, Feb 21, 2020 at 3:53 PM Li-Wen Hsu <lw...@freebsd.org> wrote:
On Sat, Feb 22, 2020 at 4:58 AM Antoine Brodin <anto...@freebsd.org> wrote:
On Thu, Feb 20, 2020 at 4:01 AM Hiroki Sato <h...@freebsd.org> wrote:
Author: hrs
Date: Thu Feb 20 03:01:27 2020
New Revision: 358152
URL: https://svnweb.freebsd.org/changeset/base/358152

Log:
   Improve performance of "read" built-in command when using a seekable
   fd.

   The read built-in command calls read(2) with a 1-byte buffer because
   newline characters need to be detected even on a byte stream which
   comes from a non-seekable file descriptor.  Because of this, the
   following script calls >6,000 read(2) to show a 6KiB file:

    while read IN; do echo "$IN"; done < /COPYRIGHT

   When the input byte stream is seekable, it is possible to read a data
   block and then reposition the file pointer to where a newline
   character found.  This change adds a small buffer to do this and
   reduces the number of read(2) calls.

   Theoretically, multiple built-in commands reading the same seekable
   byte stream in a single pipe chain can share the buffer.  However,
   this change just makes a single invocation of the read built-in
   allocate a buffer and deallocate it every time for simplicity.
   Although this causes read(2) to read the same regions multiple times,
   the performance penalty should be small compared to the reduction of
   read(2) calls.

   Reviewed by:          jilles
   MFC after:            1 week
   Differential Revision:        https://reviews.freebsd.org/D23747
This seems to be broken on at least i386.
Please either fix or revert.

Antoine (with hat: portmgr)
Could you provide more detail?  I'm worried because I didn't see
related regression from the recent test results. We may need to add
more test against the breakage you mentioned.

This trivially failed with the example in the commit message; only the
first line would be output. It also triggered a failure of
functional_test:read2 in /usr/tests/bin/sh/builtins on i386 (and all
of the other platforms with a 32-bit size_t), which would exit with a
non-zero status code.

I tested and deployed the fix suggested by cem@ as r358235 by just
making residue an off_t,

This is an example case of why it is important to keep the i386 port building and running. If we don't have a 32 bit port that is easy to build and test many bugs like these can easily go through.

Cheers,

Pedro.

_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to