kqueue
Hi,
On the WaitEventSet thread I posted a small patch to add kqueue
support[1]/messages/by-id/CAEepm=1dZ_mC+V3YtB79zf27280nign8MKOLxy2FKhvc1RzN=g@mail.gmail.com. Since then I peeked at how some other software[2]https://github.com/libevent/libevent/commit/5602e451ce872d7d60c640590113c5a81c3fc389
interacts with kqueue and discovered that there are platforms
including NetBSD where kevent.udata is an intptr_t instead of a void
*. Here's a version which should compile there. Would any NetBSD
user be interested in testing this? (An alternative would be to make
configure to test for this with some kind of AC_COMPILE_IFELSE
incantation but the steamroller cast is simpler.)
[1]: /messages/by-id/CAEepm=1dZ_mC+V3YtB79zf27280nign8MKOLxy2FKhvc1RzN=g@mail.gmail.com
[2]: https://github.com/libevent/libevent/commit/5602e451ce872d7d60c640590113c5a81c3fc389
--
Thomas Munro
http://www.enterprisedb.com
Attachments:
kqueue-v2.patchapplication/octet-stream; name=kqueue-v2.patchDownload+249-6
On Tue, Mar 29, 2016 at 7:53 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
On the WaitEventSet thread I posted a small patch to add kqueue
support[1]. Since then I peeked at how some other software[2]
interacts with kqueue and discovered that there are platforms
including NetBSD where kevent.udata is an intptr_t instead of a void
*. Here's a version which should compile there. Would any NetBSD
user be interested in testing this? (An alternative would be to make
configure to test for this with some kind of AC_COMPILE_IFELSE
incantation but the steamroller cast is simpler.)
Did you code this up blind or do you have a NetBSD machine yourself?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2016-04-21 14:15:53 -0400, Robert Haas wrote:
On Tue, Mar 29, 2016 at 7:53 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:On the WaitEventSet thread I posted a small patch to add kqueue
support[1]. Since then I peeked at how some other software[2]
interacts with kqueue and discovered that there are platforms
including NetBSD where kevent.udata is an intptr_t instead of a void
*. Here's a version which should compile there. Would any NetBSD
user be interested in testing this? (An alternative would be to make
configure to test for this with some kind of AC_COMPILE_IFELSE
incantation but the steamroller cast is simpler.)Did you code this up blind or do you have a NetBSD machine yourself?
RMT, what do you think, should we try to get this into 9.6? It's
feasible that the performance problem 98a64d0bd713c addressed is also
present on free/netbsd.
- Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Apr 21, 2016 at 2:22 PM, Andres Freund <andres@anarazel.de> wrote:
On 2016-04-21 14:15:53 -0400, Robert Haas wrote:
On Tue, Mar 29, 2016 at 7:53 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:On the WaitEventSet thread I posted a small patch to add kqueue
support[1]. Since then I peeked at how some other software[2]
interacts with kqueue and discovered that there are platforms
including NetBSD where kevent.udata is an intptr_t instead of a void
*. Here's a version which should compile there. Would any NetBSD
user be interested in testing this? (An alternative would be to make
configure to test for this with some kind of AC_COMPILE_IFELSE
incantation but the steamroller cast is simpler.)Did you code this up blind or do you have a NetBSD machine yourself?
RMT, what do you think, should we try to get this into 9.6? It's
feasible that the performance problem 98a64d0bd713c addressed is also
present on free/netbsd.
My personal opinion is that it would be a reasonable thing to do if
somebody can demonstrate that it actually solves a real problem.
Absent that, I don't think we should rush it in.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas wrote:
On Thu, Apr 21, 2016 at 2:22 PM, Andres Freund <andres@anarazel.de> wrote:
On 2016-04-21 14:15:53 -0400, Robert Haas wrote:
On Tue, Mar 29, 2016 at 7:53 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:On the WaitEventSet thread I posted a small patch to add kqueue
support[1]. Since then I peeked at how some other software[2]
interacts with kqueue and discovered that there are platforms
including NetBSD where kevent.udata is an intptr_t instead of a void
*. Here's a version which should compile there. Would any NetBSD
user be interested in testing this? (An alternative would be to make
configure to test for this with some kind of AC_COMPILE_IFELSE
incantation but the steamroller cast is simpler.)Did you code this up blind or do you have a NetBSD machine yourself?
RMT, what do you think, should we try to get this into 9.6? It's
feasible that the performance problem 98a64d0bd713c addressed is also
present on free/netbsd.My personal opinion is that it would be a reasonable thing to do if
somebody can demonstrate that it actually solves a real problem.
Absent that, I don't think we should rush it in.
My first question is whether there are platforms that use kqueue on
which the WaitEventSet stuff proves to be a bottleneck. I vaguely
recall that MacOS X in particular doesn't scale terribly well for other
reasons, and I don't know if anybody runs *BSD in large machines.
On the other hand, there's plenty of hackers running their laptops on
MacOS X these days, so presumably any platform dependent problem would
be discovered quickly enough. As for NetBSD, it seems mostly a fringe
platform, doesn't it? We would discover serious dependency problems
quickly enough on the buildfarm ... except that the only netbsd
buildfarm member hasn't reported in over two weeks.
Am I mistaken in any of these points?
(Our coverage of the BSD platforms leaves much to be desired FWIW.)
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Apr 21, 2016 at 3:31 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Robert Haas wrote:
On Thu, Apr 21, 2016 at 2:22 PM, Andres Freund <andres@anarazel.de> wrote:
On 2016-04-21 14:15:53 -0400, Robert Haas wrote:
On Tue, Mar 29, 2016 at 7:53 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:On the WaitEventSet thread I posted a small patch to add kqueue
support[1]. Since then I peeked at how some other software[2]
interacts with kqueue and discovered that there are platforms
including NetBSD where kevent.udata is an intptr_t instead of a void
*. Here's a version which should compile there. Would any NetBSD
user be interested in testing this? (An alternative would be to make
configure to test for this with some kind of AC_COMPILE_IFELSE
incantation but the steamroller cast is simpler.)Did you code this up blind or do you have a NetBSD machine yourself?
RMT, what do you think, should we try to get this into 9.6? It's
feasible that the performance problem 98a64d0bd713c addressed is also
present on free/netbsd.My personal opinion is that it would be a reasonable thing to do if
somebody can demonstrate that it actually solves a real problem.
Absent that, I don't think we should rush it in.My first question is whether there are platforms that use kqueue on
which the WaitEventSet stuff proves to be a bottleneck. I vaguely
recall that MacOS X in particular doesn't scale terribly well for other
reasons, and I don't know if anybody runs *BSD in large machines.On the other hand, there's plenty of hackers running their laptops on
MacOS X these days, so presumably any platform dependent problem would
be discovered quickly enough. As for NetBSD, it seems mostly a fringe
platform, doesn't it? We would discover serious dependency problems
quickly enough on the buildfarm ... except that the only netbsd
buildfarm member hasn't reported in over two weeks.Am I mistaken in any of these points?
(Our coverage of the BSD platforms leaves much to be desired FWIW.)
My impression is that the Linux problem only manifested itself on
large machines. I might be wrong about that. But if that's true,
then we might not see regressions on other platforms just because
people aren't running those operating systems on big enough hardware.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2016-04-21 14:25:06 -0400, Robert Haas wrote:
On Thu, Apr 21, 2016 at 2:22 PM, Andres Freund <andres@anarazel.de> wrote:
On 2016-04-21 14:15:53 -0400, Robert Haas wrote:
On Tue, Mar 29, 2016 at 7:53 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:On the WaitEventSet thread I posted a small patch to add kqueue
support[1]. Since then I peeked at how some other software[2]
interacts with kqueue and discovered that there are platforms
including NetBSD where kevent.udata is an intptr_t instead of a void
*. Here's a version which should compile there. Would any NetBSD
user be interested in testing this? (An alternative would be to make
configure to test for this with some kind of AC_COMPILE_IFELSE
incantation but the steamroller cast is simpler.)Did you code this up blind or do you have a NetBSD machine yourself?
RMT, what do you think, should we try to get this into 9.6? It's
feasible that the performance problem 98a64d0bd713c addressed is also
present on free/netbsd.My personal opinion is that it would be a reasonable thing to do if
somebody can demonstrate that it actually solves a real problem.
Absent that, I don't think we should rush it in.
On linux you needed a 2 socket machine to demonstrate the problem, but
both old ones (my 2009 workstation) and new ones were sufficient. I'd be
surprised if the situation on freebsd is any better, except that you
might hit another scalability bottleneck earlier.
I doubt there's many real postgres instances operating on bigger
hardware on freebsd, with sufficient throughput to show the problem. So
I think the argument for including is more along trying to be "nice" to
more niche-y OSs.
I really don't have any opinion either way.
- Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Apr 22, 2016 at 12:21 PM, Andres Freund <andres@anarazel.de> wrote:
On 2016-04-21 14:25:06 -0400, Robert Haas wrote:
On Thu, Apr 21, 2016 at 2:22 PM, Andres Freund <andres@anarazel.de> wrote:
On 2016-04-21 14:15:53 -0400, Robert Haas wrote:
On Tue, Mar 29, 2016 at 7:53 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:On the WaitEventSet thread I posted a small patch to add kqueue
support[1]. Since then I peeked at how some other software[2]
interacts with kqueue and discovered that there are platforms
including NetBSD where kevent.udata is an intptr_t instead of a void
*. Here's a version which should compile there. Would any NetBSD
user be interested in testing this? (An alternative would be to make
configure to test for this with some kind of AC_COMPILE_IFELSE
incantation but the steamroller cast is simpler.)Did you code this up blind or do you have a NetBSD machine yourself?
RMT, what do you think, should we try to get this into 9.6? It's
feasible that the performance problem 98a64d0bd713c addressed is also
present on free/netbsd.My personal opinion is that it would be a reasonable thing to do if
somebody can demonstrate that it actually solves a real problem.
Absent that, I don't think we should rush it in.On linux you needed a 2 socket machine to demonstrate the problem, but
both old ones (my 2009 workstation) and new ones were sufficient. I'd be
surprised if the situation on freebsd is any better, except that you
might hit another scalability bottleneck earlier.I doubt there's many real postgres instances operating on bigger
hardware on freebsd, with sufficient throughput to show the problem. So
I think the argument for including is more along trying to be "nice" to
more niche-y OSs.
What has BSD ever done for us?! (Joke...)
I vote to leave this patch in the next commitfest where it is, and
reconsider if someone shows up with a relevant problem report on large
systems. I can't see any measurable performance difference on a 4
core laptop running FreeBSD 10.3. Maybe kqueue will make more
difference even on smaller systems in future releases if we start
using big wait sets for distributed/asynchronous work, in-core
pooling/admission control etc.
Here's a new version of the patch that fixes some stupid bugs. I have
run regression tests and some basic sanity checks on OSX 10.11.4,
FreeBSD 10.3, NetBSD 7.0 and OpenBSD 5.8. There is still room to make
an improvement that would drop the syscall from AddWaitEventToSet and
ModifyWaitEvent, compressing wait set modifications and waiting into a
single syscall (kqueue's claimed advantage over the competition).
While doing that I discovered that unpatched master doesn't actually
build on recent NetBSD systems because our static function strtoi
clashes with a non-standard libc function of the same name[1]http://netbsd.gw.com/cgi-bin/man-cgi?strtoi++NetBSD-current declared
in inttypes.h. Maybe we should rename it, like in the attached?
[1]: http://netbsd.gw.com/cgi-bin/man-cgi?strtoi++NetBSD-current
--
Thomas Munro
http://www.enterprisedb.com
On 2016-04-22 20:39:27 +1200, Thomas Munro wrote:
I vote to leave this patch in the next commitfest where it is, and
reconsider if someone shows up with a relevant problem report on large
systems.
Sounds good!
Here's a new version of the patch that fixes some stupid bugs. I have
run regression tests and some basic sanity checks on OSX 10.11.4,
FreeBSD 10.3, NetBSD 7.0 and OpenBSD 5.8. There is still room to make
an improvement that would drop the syscall from AddWaitEventToSet and
ModifyWaitEvent, compressing wait set modifications and waiting into a
single syscall (kqueue's claimed advantage over the competition).
I find that not to be particularly interesting, and would rather want to
avoid adding complexity for it.
While doing that I discovered that unpatched master doesn't actually
build on recent NetBSD systems because our static function strtoi
clashes with a non-standard libc function of the same name[1] declared
in inttypes.h. Maybe we should rename it, like in the attached?
Yuck. That's a new function they introduced? That code hasn't changed in
a while....
Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Apr 23, 2016 at 4:36 AM, Andres Freund <andres@anarazel.de> wrote:
On 2016-04-22 20:39:27 +1200, Thomas Munro wrote:
While doing that I discovered that unpatched master doesn't actually
build on recent NetBSD systems because our static function strtoi
clashes with a non-standard libc function of the same name[1] declared
in inttypes.h. Maybe we should rename it, like in the attached?Yuck. That's a new function they introduced? That code hasn't changed in
a while....
Yes, according to the man page it appeared in NetBSD 7.0. That was
released in September 2015, and our buildfarm has only NetBSD 5.x
systems. I see that the maintainers of the NetBSD pg package deal
with this with a preprocessor kludge:
What is the policy for that kind of thing -- do nothing until someone
cares enough about the platform to supply a buildfarm animal?
--
Thomas Munro
http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Thomas Munro wrote:
On Sat, Apr 23, 2016 at 4:36 AM, Andres Freund <andres@anarazel.de> wrote:
On 2016-04-22 20:39:27 +1200, Thomas Munro wrote:
While doing that I discovered that unpatched master doesn't actually
build on recent NetBSD systems because our static function strtoi
clashes with a non-standard libc function of the same name[1] declared
in inttypes.h. Maybe we should rename it, like in the attached?Yuck. That's a new function they introduced? That code hasn't changed in
a while....Yes, according to the man page it appeared in NetBSD 7.0. That was
released in September 2015, and our buildfarm has only NetBSD 5.x
systems. I see that the maintainers of the NetBSD pg package deal
with this with a preprocessor kludge:What is the policy for that kind of thing -- do nothing until someone
cares enough about the platform to supply a buildfarm animal?
Well, if the platform is truly alive, we would have gotten complaints
already. Since we haven't, maybe nobody cares, so why should we? I
would rename our function nonetheless FWIW; the name seems far too
generic to me. pg_strtoi?
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2016-04-23 10:12:12 +1200, Thomas Munro wrote:
What is the policy for that kind of thing -- do nothing until someone
cares enough about the platform to supply a buildfarm animal?
I think we should fix it, I just want to make sure we understand why the
error is appearing now. Since we now do...
- Andres
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2016-04-22 19:25:06 -0300, Alvaro Herrera wrote:
Since we haven't, maybe nobody cares, so why should we?
I guess it's to a good degree because netbsd has pg packages, and it's
fixed there?
would rename our function nonetheless FWIW; the name seems far too
generic to me.
Yea.
pg_strtoi?
I think that's what Thomas did upthread. Are you taking this one then?
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Thomas Munro <thomas.munro@enterprisedb.com> writes:
On Sat, Apr 23, 2016 at 4:36 AM, Andres Freund <andres@anarazel.de> wrote:
On 2016-04-22 20:39:27 +1200, Thomas Munro wrote:
While doing that I discovered that unpatched master doesn't actually
build on recent NetBSD systems because our static function strtoi
clashes with a non-standard libc function of the same name[1] declared
in inttypes.h. Maybe we should rename it, like in the attached?
Yuck. That's a new function they introduced? That code hasn't changed in
a while....
Yes, according to the man page it appeared in NetBSD 7.0. That was
released in September 2015, and our buildfarm has only NetBSD 5.x
systems. I see that the maintainers of the NetBSD pg package deal
with this with a preprocessor kludge:
What is the policy for that kind of thing -- do nothing until someone
cares enough about the platform to supply a buildfarm animal?
There's no set policy, but certainly a promise to put up a buildfarm
animal would establish that somebody actually cares about keeping
Postgres running on the platform. Without one, we might fix a specific
problem when reported, but we'd have no way to know about new problems.
Rooting through that patches directory reveals quite a number of
random-looking patches, most of which we certainly wouldn't take
without a lot more than zero explanation. It's hard to tell which
are actually needed, but at least some don't seem to have anything
to do with building for NetBSD.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
pg_strtoi?
I think that's what Thomas did upthread. Are you taking this one then?
I'd go with just "strtoint". We have "strtoint64" elsewhere.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
pg_strtoi?
I think that's what Thomas did upthread. Are you taking this one then?
I'd go with just "strtoint". We have "strtoint64" elsewhere.
For closure of this subthread: this rename was committed by Tom as
0ab3595e5bb5.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Jun 3, 2016 at 4:02 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
pg_strtoi?
I think that's what Thomas did upthread. Are you taking this one then?
I'd go with just "strtoint". We have "strtoint64" elsewhere.
For closure of this subthread: this rename was committed by Tom as
0ab3595e5bb5.
Thanks. And here is a new version of the kqueue patch. The previous
version doesn't apply on top of recent commit
a3b30763cc8686f5b4cd121ef0bf510c1533ac22, which sprinkled some
MAXALIGN macros nearby. I've now done the same thing with the kevent
struct because it's cheap, uniform with the other cases and could
matter on some platforms for the same reason.
It's in the September commitfest here: https://commitfest.postgresql.org/10/597/
--
Thomas Munro
http://www.enterprisedb.com
Attachments:
kqueue-v4.patchapplication/octet-stream; name=kqueue-v4.patchDownload+273-6
On 2016-06-03 01:45, Thomas Munro wrote:
On Fri, Jun 3, 2016 at 4:02 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
pg_strtoi?
I think that's what Thomas did upthread. Are you taking this one then?
I'd go with just "strtoint". We have "strtoint64" elsewhere.
For closure of this subthread: this rename was committed by Tom as
0ab3595e5bb5.Thanks. And here is a new version of the kqueue patch. The previous
version doesn't apply on top of recent commit
a3b30763cc8686f5b4cd121ef0bf510c1533ac22, which sprinkled some
MAXALIGN macros nearby. I've now done the same thing with the kevent
struct because it's cheap, uniform with the other cases and could
matter on some platforms for the same reason.
I've tested and reviewed this, and it looks good to me, other than this
part:
+ /*
+ * kevent guarantees that the change list has been processed in the
EINTR
+ * case. Here we are only applying a change list so EINTR counts as
+ * success.
+ */this doesn't seem to be guaranteed on old versions of FreeBSD or any
other BSD flavors, so I don't think it's a good idea to bake the
assumption into this code. Or what do you think?
.m
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Sep 7, 2016 at 12:32 AM, Marko Tiikkaja <marko@joh.to> wrote:
I've tested and reviewed this, and it looks good to me, other than this
part:+ /* + * kevent guarantees that the change list has been processed in the EINTR + * case. Here we are only applying a change list so EINTR counts as + * success. + */this doesn't seem to be guaranteed on old versions of FreeBSD or any other
BSD flavors, so I don't think it's a good idea to bake the assumption into
this code. Or what do you think?
Thanks for the testing and review!
Hmm. Well spotted. I wrote that because the man page from FreeBSD 10.3 says:
When kevent() call fails with EINTR error, all changes in the changelist
have been applied.
This sentence is indeed missing from the OpenBSD, NetBSD and OSX man
pages. It was introduced by FreeBSD commit r280818[1]https://svnweb.freebsd.org/base?view=revision&revision=280818 which made
kevent a Pthread cancellation point. I investigated whether it is
also true in older FreeBSD and the rest of the BSD family. I believe
the answer is yes.
1. That commit doesn't do anything that would change the situation:
it just adds thread cancellation wrapper code to libc and libthr which
exits under certain conditions but otherwise lets EINTR through to the
caller. So I think the new sentence is documentation of the existing
behaviour of the syscall.
2. I looked at the code in FreeBSD 4.1[2]https://github.com/freebsd/freebsd/blob/release/4.1.0/sys/kern/kern_event.c (the original kqueue
implementation from which all others derive) and the four modern
OSes[3]https://github.com/freebsd/freebsd/blob/master/sys/kern/kern_event.c[4]https://github.com/IIJ-NetBSD/netbsd-src/blob/master/sys/kern/kern_event.c[5]https://github.com/openbsd/src/blob/master/sys/kern/kern_event.c[6]https://github.com/opensource-apple/xnu/blob/master/bsd/kern/kern_event.c. They vary a bit but in all cases, the first place
that can produce EINTR appears to be in kqueue_scan when the
(variously named) kernel sleep routine is invoked, which can return
EINTR or ERESTART (later translated to EINTR because kevent doesn't
support restarting). That comes after all changes have been applied.
In fact it's unreachable if nevents is 0: OSX doesn't call kqueue_scan
in that case, and the others return early from kqueue_scan in that
case.
3. An old email[7]http://marc.info/?l=freebsd-arch&m=98147346707952&w=2 from Jonathan Lemon (creator of kqueue) seems to
support that at least in respect of ancient FreeBSD. He wrote:
"Technically, an EINTR is returned when a signal interrupts the
process after it goes to sleep (that is, after it calls tsleep). So
if (as an example) you call kevent() with a zero valued timespec,
you'll never get EINTR, since there's no possibility of it sleeping."
So if I've understood correctly, what I wrote in the v4 patch is
universally true, but it's also moot in this case: kevent cannot fail
with errno == EINTR because nevents == 0. On that basis, here is a
new version with the comment and special case for EINTR removed.
[1]: https://svnweb.freebsd.org/base?view=revision&revision=280818
[2]: https://github.com/freebsd/freebsd/blob/release/4.1.0/sys/kern/kern_event.c
[3]: https://github.com/freebsd/freebsd/blob/master/sys/kern/kern_event.c
[4]: https://github.com/IIJ-NetBSD/netbsd-src/blob/master/sys/kern/kern_event.c
[5]: https://github.com/openbsd/src/blob/master/sys/kern/kern_event.c
[6]: https://github.com/opensource-apple/xnu/blob/master/bsd/kern/kern_event.c
[7]: http://marc.info/?l=freebsd-arch&m=98147346707952&w=2
--
Thomas Munro
http://www.enterprisedb.com
Attachments:
kqueue-v5.patchapplication/octet-stream; name=kqueue-v5.patchDownload+268-6
So, if I've understood correctly, the purpose of this patch is to
improve performance on a multi-CPU system, which has the kqueue()
function. Most notably, FreeBSD?
I launched a FreeBSD 10.3 instance on Amazon EC2 (ami-e0682b80), on a
m4.10xlarge instance. That's a 40 core system, biggest available, I
believe. I built PostgreSQL master on it, and ran pgbench to benchmark:
pgbench -i -s 200 postgres
pgbench -M prepared -j 36 -c 36 -S postgres -T20 -P1
I set shared_buffers to 10 GB, so that the whole database fits in cache.
I tested that with and without kqueue-v5.patch
Result: I don't see any difference in performance. pgbench reports
between 80,000 and 97,000 TPS, with or without the patch:
[ec2-user@ip-172-31-17-174 ~/postgresql]$ ~/pgsql-install/bin/pgbench -M
prepared -j 36 -c 36 -S postgres -T20 -P1
starting vacuum...end.
progress: 1.0 s, 94537.1 tps, lat 0.368 ms stddev 0.145
progress: 2.0 s, 96745.9 tps, lat 0.368 ms stddev 0.143
progress: 3.0 s, 93870.1 tps, lat 0.380 ms stddev 0.146
progress: 4.0 s, 89482.9 tps, lat 0.399 ms stddev 0.146
progress: 5.0 s, 87815.0 tps, lat 0.406 ms stddev 0.148
progress: 6.0 s, 86415.5 tps, lat 0.413 ms stddev 0.145
progress: 7.0 s, 86011.0 tps, lat 0.415 ms stddev 0.147
progress: 8.0 s, 84923.0 tps, lat 0.420 ms stddev 0.147
progress: 9.0 s, 84596.6 tps, lat 0.422 ms stddev 0.146
progress: 10.0 s, 84537.7 tps, lat 0.422 ms stddev 0.146
progress: 11.0 s, 83910.5 tps, lat 0.425 ms stddev 0.150
progress: 12.0 s, 83738.2 tps, lat 0.426 ms stddev 0.150
progress: 13.0 s, 83837.5 tps, lat 0.426 ms stddev 0.147
progress: 14.0 s, 83578.4 tps, lat 0.427 ms stddev 0.147
progress: 15.0 s, 83609.5 tps, lat 0.427 ms stddev 0.148
progress: 16.0 s, 83423.5 tps, lat 0.428 ms stddev 0.151
progress: 17.0 s, 83318.2 tps, lat 0.428 ms stddev 0.149
progress: 18.0 s, 82992.7 tps, lat 0.430 ms stddev 0.149
progress: 19.0 s, 83155.9 tps, lat 0.429 ms stddev 0.151
progress: 20.0 s, 83209.0 tps, lat 0.429 ms stddev 0.152
transaction type: <builtin: select only>
scaling factor: 200
query mode: prepared
number of clients: 36
number of threads: 36
duration: 20 s
number of transactions actually processed: 1723759
latency average = 0.413 ms
latency stddev = 0.149 ms
tps = 86124.484867 (including connections establishing)
tps = 86208.458034 (excluding connections establishing)
Is this test setup reasonable? I know very little about FreeBSD, I'm
afraid, so I don't know how to profile or test that further than that.
If there's no measurable difference in performance, between kqueue() and
poll(), I think we should forget about this. If there's a FreeBSD hacker
out there that can demonstrate better results, I'm all for committing
this, but I'm reluctant to add code if no-one can show the benefit.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
