Date: Tue, 3 Jul 2018 00:53:28 +1200 From: Thomas Munro <munro@ip9.org> To: freebsd-hackers@freebsd.org Subject: Patching setproctitle() to go faster, for PostgreSQL Message-ID: <CADLWmXWCj_WgYfQfzUL_aFLYYqezmbivzF=gKyk%2B6JqPcBvbXQ@mail.gmail.com>
next in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
Hi FreeBSD hackers,
PostgreSQL's update_process_titles setting, which defaults to on,
slows busy databases down quite noticeably on FreeBSD and so a lot of
people turn it off. Having that information in ps/top/htop is nice,
so I don't like turning it off. It was a rainy Sunday here yesterday
so I decided to hack on that. I wrote an experimental patch,
attached, that essentially reverts to the ancient syscall-free BSD
behaviour (?) on a per-process basis, for processes that are
update-heavy. That raises some interesting questions about torn
reads, I admit...
I didn't see much difference in "pgbench" on my laptop, but on an AWS
m4.10xlarge (40 vCPU) machine I could easily see a difference. I
think there is probably a contention effect somewhere that gets worse
with more concurrency.
pgbench -i -s 10 postgres
pgbench -T60 -c 40 -j 40 -S -M prepared postgres
Stock 11.2, update_process_titles = on: 472873 TPS (default)
Stock 11.2, update_process_titles = off: 539391 TPS (~13% faster
than default)
Patched 11.2, update_process_titles = on: 519733 TPS (~10% faster
than default)
I'd be grateful for any feedback.
Thanks,
Thomas Munro
[-- Attachment #2 --]
From ef938f5acfa38d20d5da630edb18387add116050 Mon Sep 17 00:00:00 2001
From: Thomas Munro <munro@ip9.org>
Date: Tue, 3 Jul 2018 00:30:48 +1200
Subject: [PATCH] Speed up setproctitle() for frequent callers.
Some applications, notably PostgreSQL, want to call setproctitle()
very often. It's slow. Provide an alternative cheap way of updating
process titles without making any syscalls, instead requiring other
processes (top, ps etc) to do a bit more work to retrieve the data.
This uses a pre-existing code path inherited from ancient BSD, which
always did it that way. Switch to the fast update strategy after 5
calls to setproctitle() from any given process.
Thomas Munro
---
lib/libc/gen/setproctitle.c | 26 +++++++++++++++++++++-----
sys/kern/kern_proc.c | 19 ++++++++++++++-----
usr.bin/top/machine.c | 1 -
3 files changed, 35 insertions(+), 11 deletions(-)
diff --git a/lib/libc/gen/setproctitle.c b/lib/libc/gen/setproctitle.c
index fd87c350715..ff2e533bd53 100644
--- a/lib/libc/gen/setproctitle.c
+++ b/lib/libc/gen/setproctitle.c
@@ -55,6 +55,8 @@ struct old_ps_strings {
#define SPT_BUFSIZE 2048 /* from other parts of sendmail */
+#define MAX_SLOW_UPDATES 5 /* threshold for switching to fast path */
+
void
setproctitle(const char *fmt, ...)
{
@@ -64,6 +66,8 @@ setproctitle(const char *fmt, ...)
static char **oargv, *kbuf;
static int oargc = -1;
static char *nargv[2] = { NULL, NULL };
+ static int fast_update = 0;
+ static int calls = 0;
char **nargvp;
int nargc;
int i;
@@ -91,6 +95,16 @@ setproctitle(const char *fmt, ...)
if (fmt) {
buf[SPT_BUFSIZE - 1] = '\0';
+ if (!fast_update && ++calls >= MAX_SLOW_UPDATES) {
+ /* tell the kernel to start looking in user-space */
+ oid[0] = CTL_KERN;
+ oid[1] = KERN_PROC;
+ oid[2] = KERN_PROC_ARGS;
+ oid[3] = getpid();
+ sysctl(oid, 4, 0, 0, "", 0);
+ fast_update = 1;
+ }
+
if (fmt[0] == '-') {
/* skip program name prefix */
fmt++;
@@ -119,11 +133,13 @@ setproctitle(const char *fmt, ...)
va_end(ap);
/* Set the title into the kernel cached command line */
- oid[0] = CTL_KERN;
- oid[1] = KERN_PROC;
- oid[2] = KERN_PROC_ARGS;
- oid[3] = getpid();
- sysctl(oid, 4, 0, 0, kbuf, strlen(kbuf) + 1);
+ if (!fast_update) {
+ oid[0] = CTL_KERN;
+ oid[1] = KERN_PROC;
+ oid[2] = KERN_PROC_ARGS;
+ oid[3] = getpid();
+ sysctl(oid, 4, 0, 0, kbuf, strlen(kbuf) + 1);
+ }
if (ps_strings == NULL) {
len = sizeof(ul_ps_strings);
diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c
index 11dfd2f44eb..0280a2c706b 100644
--- a/sys/kern/kern_proc.c
+++ b/sys/kern/kern_proc.c
@@ -1988,11 +1988,20 @@ sysctl_kern_proc_args(SYSCTL_HANDLER_ARGS)
if (req->newlen > ps_arg_cache_limit - sizeof(struct pargs))
return (ENOMEM);
- newpa = pargs_alloc(req->newlen);
- error = SYSCTL_IN(req, newpa->ar_args, req->newlen);
- if (error != 0) {
- pargs_free(newpa);
- return (error);
+
+ if (req->newlen == 0) {
+ /*
+ * Clear the argument pointer, so that we'll fetch arguments
+ * with proc_getargv() until further notice.
+ */
+ newpa = NULL;
+ } else {
+ newpa = pargs_alloc(req->newlen);
+ error = SYSCTL_IN(req, newpa->ar_args, req->newlen);
+ if (error != 0) {
+ pargs_free(newpa);
+ return (error);
+ }
}
PROC_LOCK(p);
pa = p->p_args;
diff --git a/usr.bin/top/machine.c b/usr.bin/top/machine.c
index c44dec264a5..f0c7b8c8410 100644
--- a/usr.bin/top/machine.c
+++ b/usr.bin/top/machine.c
@@ -950,7 +950,6 @@ format_next_process(struct handle * xhandle, char *(*get_userid)(int), int flags
}
} else {
if (pp->ki_flag & P_SYSTEM ||
- pp->ki_args == NULL ||
(args = kvm_getargv(kd, pp, cmdlen)) == NULL ||
!(*args)) {
if (ps.thread && pp->ki_flag & P_HADTHREADS &&
--
2.17.0
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CADLWmXWCj_WgYfQfzUL_aFLYYqezmbivzF=gKyk%2B6JqPcBvbXQ>