Date: Mon, 26 Nov 2012 23:05:05 +1100
Message-ID: <20121126230505.GA2095@hili.localdomain>
From: Peter Wang <novalazy@gmail.com>
To: Tomi Ollila <tomi.ollila@iki.fi>
Subject: Re: [PATCH v2 05/20] insert: copy stdin to Maildir tmp file
In-Reply-To: <m2txscitya.fsf@guru.guru-group.fi>
References: <1353806206-29133-1-git-send-email-novalazy@gmail.com>
	<1353806206-29133-6-git-send-email-novalazy@gmail.com>
	<m2txscitya.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
Cc: notmuch@notmuchmail.org
Precedence: list

On Mon, 26 Nov 2012 11:39:41 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Sun, Nov 25 2012, Peter Wang <novalazy@gmail.com> wrote:
> 
> > Read the new message from standard input into the Maildir tmp file.
> > ---
> 
> There are a few issues that gort my attention in this particular function:
> 
> 
> >  notmuch-insert.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 47 insertions(+), 4 deletions(-)
> >
> > diff --git a/notmuch-insert.c b/notmuch-insert.c
> > index 371fb47..88e8533 100644
> > --- a/notmuch-insert.c
> > +++ b/notmuch-insert.c
> > @@ -94,6 +94,47 @@ maildir_open_tmp_file (void *ctx, const char *dir,
> >      return fd;
> >  }
> >  
> > +/* Copy the contents of fdin into fdout. */
> > +static notmuch_bool_t
> > +copy_fd_data (int fdin, int fdout)
> > +{
> > +    char buf[4096];
> 
> Copying in 4k blocks is slow when at least when doing file to file copy.
> Also socket buffers can often hold much more data. When reading from
> network and saving to file (in low-load machine) this is OK, but otherwise
> something like 64k buffer works better(*).
> 
> (*) Now that I said it I have to measure this yet another time ;)

I get up to a whopping 10 ms faster copy of a 100 MiB file by increasing
the buffer size to 8192 bytes or more (standalone program, stdin to
disk, no fsync, no xapian).  Feel free to fiddle with it after it's
pushed, if you think it's worthwhile.

> You're claiming in function name & and its description that this is more
> "generic" copy function -- yet error message speaks about 'standard input'.

I'll rename it.

Peter