Re: svn commit: r109866 - /httpd/httpd/trunk/modules/loggers/mod_log_config.c
On Mon, 6 Dec 2004, Paul Querna wrote: > Having looked at the writev() implementation in FreeBSD 5.2, it will Not all platforms have writev(). I hope we stil have that APR flag for that ?! Dw
Re: svn commit: r109866 - /httpd/httpd/trunk/modules/loggers/mod_log_config.c
On Dec 6, 2004, at 7:53 PM, Paul Querna wrote: I do not agree that using writev() would allow multiple children from scribbling over each other's log entries. I have not been able to cause this to happen on my local machines. You might want to consider what happens on all of the not so recent operating systems that run Apache, especially those that don't even implement writev. See what happens when APR_HAVE_WRITEV is not defined to 1. Roy
Re: svn commit: r109866 - /httpd/httpd/trunk/modules/loggers/mod_log_config.c
Roy T. Fielding wrote: -1 (veto) -- the message is copied to a single buffer for the write because that is the only way to guarantee an atomic append under Unix without locks, thus preventing multiple children from scribbling over each other's log entries. Please revert this change ASAP. Roy I have committed r110069 which reverts r109866, based on your -1 vote. I do not agree that using writev() would allow multiple children from scribbling over each other's log entries. I have not been able to cause this to happen on my local machines. Having looked at the writev() implementation in FreeBSD 5.2, it will lock the entire range of a file to be written with writev, and will not allow another process to write over that range. The Linux 2.6 implementation uses similar locking. According to IEEE Std 1003.1: http://www.opengroup.org/onlinepubs/009695399/functions/writev.html "The writev() function shall be equivalent to write(), except as described below." None of the exceptions described state writev() should not operate in same atomic manner as write(). If other Unixes are not standards compliant in this regard, then I believe it would be correct to revert to the original behavoir. -Paul Querna
Re: svn commit: r109866 - /httpd/httpd/trunk/modules/loggers/mod_log_config.c
-1 (veto) -- the message is copied to a single buffer for the write because that is the only way to guarantee an atomic append under Unix without locks, thus preventing multiple children from scribbling over each other's log entries. Please revert this change ASAP. Roy On Dec 5, 2004, at 12:58 PM, Paul Querna wrote: Joe Orton wrote: On Sun, Dec 05, 2004 at 07:05:23AM -, Paul Querna wrote: Author: pquerna Date: Sat Dec 4 23:05:23 2004 New Revision: 109866 URL: http://svn.apache.org/viewcvs?view=rev&rev=109866 Log: mod_log_config.c: Use iovecs to write the log line to eliminate a memcpy IIRC, writev'ing several small blocks to a file is actually generally more expensive than doing a memcpy in userspace and calling write. Did you benchmark this to be faster/better/...? I did a local mini-benchmark of write w/ memcpy vs writev... and they came out to almost exactly the same on average with small sets of data. -Paul
Re: svn commit: r109866 - /httpd/httpd/trunk/modules/loggers/mod_log_config.c
On Sun, Dec 05, 2004 at 01:58:44PM -0700, Paul Querna wrote:
> Joe Orton wrote:
> >On Sun, Dec 05, 2004 at 07:05:23AM -, Paul Querna wrote:
> >
> >>Author: pquerna
> >>Date: Sat Dec 4 23:05:23 2004
> >>New Revision: 109866
> >>
> >>URL: http://svn.apache.org/viewcvs?view=rev&rev=109866
> >>Log:
> >>mod_log_config.c: Use iovecs to write the log line to eliminate a memcpy
> >
> >
> >IIRC, writev'ing several small blocks to a file is actually generally
> >more expensive than doing a memcpy in userspace and calling write. Did
> >you benchmark this to be faster/better/...?
> >
also that introduced a warning:
mod_log_config.c: In function `ap_default_log_writer':
mod_log_config.c:1353: warning: assignment discards qualifiers from pointer
target type
> I did a local mini-benchmark of write w/ memcpy vs writev... and they
> came out to almost exactly the same on average with small sets of data.
I remember I checked this before too... try the attached compiled with
or without -DUSE_WRITEV, copy-and-write comes out about ~10-20% faster
than writev on Linux 2.6/ext3 here for a real log vector, or did I screw
up the benchmark?
rm -f writev.out; sync; sleep 5
./writev-copy
copy+write: 7s330676.
rm -f writev.out; sync; sleep 5
./writev-copy
copy+write: 7s327580.
rm -f writev.out; sync; sleep 5
./writev-copy
copy+write: 7s360685.
rm -f writev.out; sync; sleep 5
./writev-writev
writev: 8s893524.
rm -f writev.out; sync; sleep 5
./writev-writev
writev: 8s808458.
rm -f writev.out; sync; sleep 5
./writev-writev
writev: 9s052335.
joe
#include
#include
#include
#include
#include
#include
#include
#include
char *strs[] = {
"127.0.0.1",
" ",
"-",
" ",
"-",
" ",
"[22/Jul/2003:12:09:58 +0100]",
" \"",
"GET / HTTP/1.0",
"\" ",
"200",
" ",
"1456",
"\n"
};
#define NVECS (sizeof(strs) / sizeof(char *))
#define ITERS (100)
#ifdef USE_WRITEV
static void do_writev(int fd, struct iovec *vec, size_t nvec, size_t total)
{
ssize_t ret = writev(fd, vec, nvec);
if (ret != total) {
printf("writev got %d not %u\n", ret, total);
}
}
#else
static void do_writev(int fd, struct iovec *vec, size_t nvec, size_t total)
{
char *p, *buf;
ssize_t ret;
p = buf = malloc(total);
while (nvec) {
memcpy(p, vec[0].iov_base, vec[0].iov_len);
p += vec[0].iov_len;
nvec--;
vec++;
}
ret = write(fd, buf, total);
free(buf);
if (ret != total) {
printf("write got %d not %u\n", ret, total);
}
}
#endif
#define BIG 100
#define TESTFN "./writev.out"
int main(int argc, char **argv)
{
int fd;
size_t n, total = 0;
struct iovec vecs[NVECS];
int count = 0;
struct timeval start, end, diff;
unlink(TESTFN);
fd = open(TESTFN, O_CREAT | O_WRONLY | O_TRUNC, 0644);
if (fd < 0) {
perror("open");
return 1;
}
#if 0
for (n = 1; n < 5; n++) {
strs[n] = malloc(BIG * n + 1);
memset(strs[n], 'a' + n, BIG * n);
strs[n][BIG * n] = 0;
}
#endif
for (n = 0; n < NVECS; n++) {
vecs[n].iov_base = strs[n];
vecs[n].iov_len = strlen(strs[n]);
total += vecs[n].iov_len;
}
gettimeofday(&start, NULL);
while (count++ < ITERS)
do_writev(fd, vecs, NVECS, total);
gettimeofday(&end, NULL);
timersub(&end, &start, &diff);
#ifdef USE_WRITEV
printf("writev");
#else
printf("copy+write");
#endif
printf(": %lds%06ld.\n", diff.tv_sec, diff.tv_usec);
close(fd);
return 0;
}
Re: svn commit: r109866 - /httpd/httpd/trunk/modules/loggers/mod_log_config.c
Joe Orton wrote: On Sun, Dec 05, 2004 at 07:05:23AM -, Paul Querna wrote: Author: pquerna Date: Sat Dec 4 23:05:23 2004 New Revision: 109866 URL: http://svn.apache.org/viewcvs?view=rev&rev=109866 Log: mod_log_config.c: Use iovecs to write the log line to eliminate a memcpy IIRC, writev'ing several small blocks to a file is actually generally more expensive than doing a memcpy in userspace and calling write. Did you benchmark this to be faster/better/...? I did a local mini-benchmark of write w/ memcpy vs writev... and they came out to almost exactly the same on average with small sets of data. -Paul
Re: svn commit: r109866 - /httpd/httpd/trunk/modules/loggers/mod_log_config.c
On Sun, Dec 05, 2004 at 07:05:23AM -, Paul Querna wrote:
> Author: pquerna
> Date: Sat Dec 4 23:05:23 2004
> New Revision: 109866
>
> URL: http://svn.apache.org/viewcvs?view=rev&rev=109866
> Log:
> mod_log_config.c: Use iovecs to write the log line to eliminate a memcpy
IIRC, writev'ing several small blocks to a file is actually generally
more expensive than doing a memcpy in userspace and calling write. Did
you benchmark this to be faster/better/...?
> Modified:
>httpd/httpd/trunk/modules/loggers/mod_log_config.c
>
> Modified: httpd/httpd/trunk/modules/loggers/mod_log_config.c
> Url:
> http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/loggers/mod_log_config.c?view=diff&rev=109866&p1=httpd/httpd/trunk/modules/loggers/mod_log_config.c&r1=109865&p2=httpd/httpd/trunk/modules/loggers/mod_log_config.c&r2=109866
> ==
> --- httpd/httpd/trunk/modules/loggers/mod_log_config.c(original)
> +++ httpd/httpd/trunk/modules/loggers/mod_log_config.cSat Dec 4
> 23:05:23 2004
> @@ -1343,19 +1343,18 @@
> apr_size_t len)
>
> {
> -char *str;
> -char *s;
> int i;
> apr_status_t rv;
> +struct iovec *vec;
>
> -str = apr_palloc(r->pool, len + 1);
> +vec = apr_palloc(r->pool, nelts * sizeof(struct iovec));
>
> -for (i = 0, s = str; i < nelts; ++i) {
> -memcpy(s, strs[i], strl[i]);
> -s += strl[i];
> +for (i = 0; i < nelts; ++i) {
> +vec[i].iov_base = strs[i];
> +vec[i].iov_len = strl[i];
> }
>
> -rv = apr_file_write((apr_file_t*)handle, str, &len);
> +rv = apr_file_writev((apr_file_t*)handle, vec, nelts, &i);
>
> return rv;
> }
