[PATCH v2 05/20] insert: copy stdin to Maildir tmp file
On Mon, 26 Nov 2012 11:39:41 +0200, Tomi Ollila wrote:
> On Sun, Nov 25 2012, Peter Wang 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
[PATCH v2 05/20] insert: copy stdin to Maildir tmp file
On Sun, Nov 25 2012, Peter Wang 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 ;)
> +char *p;
> +ssize_t remain;
> +ssize_t written;
> +
> +for (;;) {
> + remain = read (fdin, buf, sizeof(buf));
space between sizeof and (buf)
> + if (remain == 0)
> + break;
> + if (remain < 0) {
> + if (errno == EINTR)
> + continue;
> + fprintf (stderr, "Error: reading from standard input: %s\n",
> + strerror (errno));
> + return FALSE;
> + }
You're claiming in function name & and its description that this is more
"generic" copy function -- yet error message speaks about 'standard input'.
> +
> + p = buf;
> + do {
> + written = write (fdout, p, remain);
> + if (written == 0)
> + return FALSE;
> + if (written < 0) {
> + if (errno == EINTR)
> + continue;
> + fprintf (stderr, "Error: writing to temporary file: %s",
> + strerror (errno));
> + return FALSE;
> + }
Ditto.
> + p += written;
> + remain -= written;
> + } while (remain > 0);
> +}
> +
> +return TRUE;
> +}
> +
Tomi
Re: [PATCH v2 05/20] insert: copy stdin to Maildir tmp file
On Mon, 26 Nov 2012 11:39:41 +0200, Tomi Ollila tomi.oll...@iki.fi wrote:
On Sun, Nov 25 2012, Peter Wang noval...@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
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v2 05/20] insert: copy stdin to Maildir tmp file
Read the new message from standard input into the Maildir tmp file.
---
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];
+char *p;
+ssize_t remain;
+ssize_t written;
+
+for (;;) {
+ remain = read (fdin, buf, sizeof(buf));
+ if (remain == 0)
+ break;
+ if (remain < 0) {
+ if (errno == EINTR)
+ continue;
+ fprintf (stderr, "Error: reading from standard input: %s\n",
+strerror (errno));
+ return FALSE;
+ }
+
+ p = buf;
+ do {
+ written = write (fdout, p, remain);
+ if (written == 0)
+ return FALSE;
+ if (written < 0) {
+ if (errno == EINTR)
+ continue;
+ fprintf (stderr, "Error: writing to temporary file: %s",
+strerror (errno));
+ return FALSE;
+ }
+ p += written;
+ remain -= written;
+ } while (remain > 0);
+}
+
+return TRUE;
+}
+
static notmuch_bool_t
insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
const char *dir)
@@ -101,16 +142,18 @@ insert_message (void *ctx, notmuch_database_t *notmuch,
int fdin,
char *tmppath;
char *newpath;
int fdout;
+notmuch_bool_t ret;
fdout = maildir_open_tmp_file (ctx, dir, , );
if (fdout < 0) {
return FALSE;
}
-
-/* For now we just delete the tmp file immediately. */
+ret = copy_fd_data (fdin, fdout);
close (fdout);
-unlink (tmppath);
-return FALSE;
+if (!ret) {
+ unlink (tmppath);
+}
+return ret;
}
int
--
1.7.12.1
[PATCH v2 05/20] insert: copy stdin to Maildir tmp file
Read the new message from standard input into the Maildir tmp file.
---
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];
+char *p;
+ssize_t remain;
+ssize_t written;
+
+for (;;) {
+ remain = read (fdin, buf, sizeof(buf));
+ if (remain == 0)
+ break;
+ if (remain 0) {
+ if (errno == EINTR)
+ continue;
+ fprintf (stderr, Error: reading from standard input: %s\n,
+strerror (errno));
+ return FALSE;
+ }
+
+ p = buf;
+ do {
+ written = write (fdout, p, remain);
+ if (written == 0)
+ return FALSE;
+ if (written 0) {
+ if (errno == EINTR)
+ continue;
+ fprintf (stderr, Error: writing to temporary file: %s,
+strerror (errno));
+ return FALSE;
+ }
+ p += written;
+ remain -= written;
+ } while (remain 0);
+}
+
+return TRUE;
+}
+
static notmuch_bool_t
insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
const char *dir)
@@ -101,16 +142,18 @@ insert_message (void *ctx, notmuch_database_t *notmuch,
int fdin,
char *tmppath;
char *newpath;
int fdout;
+notmuch_bool_t ret;
fdout = maildir_open_tmp_file (ctx, dir, tmppath, newpath);
if (fdout 0) {
return FALSE;
}
-
-/* For now we just delete the tmp file immediately. */
+ret = copy_fd_data (fdin, fdout);
close (fdout);
-unlink (tmppath);
-return FALSE;
+if (!ret) {
+ unlink (tmppath);
+}
+return ret;
}
int
--
1.7.12.1
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
