spawn(): improve performance and simplify API

posix_spawn() is designed especially for this purpose, and thus it's
much more lightweight and efficient than manually fork/dup/exec-ing.

on my system, it improves the performance of spawn() by about 10x. given
that we make frequent calls to potentially multiple scripts, the
increased efficiency will add up overtime.

using posix_spawn() also simplifies the logic quite a bit, despite the
very verbose function names. however it does make cleanup a bit more
complicated.

this patch uses the linux kernel style cleanup strategy [0] (which I'm
personally not a huge fan of, but it fits this situation quite nicely)
with a "stack-like" unwinding via `goto`-s.

additionally simplify the spawn() API by taking in {read,write}fd
pointers and returning the pid instead of using some custom struct.

this coincidently also fixes #299

[0]: https://www.kernel.org/doc/html/v4.10/process/coding-style.html?highlight=goto#centralized-exiting-of-functions
This commit is contained in:
NRK 2022-06-22 09:28:07 +06:00
parent 76c2b81b60
commit 49d11f0d1f
3 changed files with 73 additions and 85 deletions

34
main.c
View File

@ -277,9 +277,9 @@ static bool check_timeouts(int *t)
static size_t get_win_title(char *buf, size_t len) static size_t get_win_title(char *buf, size_t len)
{ {
char *argv[8]; char *argv[8];
spawn_t pfd;
char w[12] = "", h[12] = "", z[12] = "", fidx[12], fcnt[12]; char w[12] = "", h[12] = "", z[12] = "", fidx[12], fcnt[12];
ssize_t n = -1; ssize_t n = -1;
int readfd;
if (wintitle.f.err || buf == NULL || len == 0) if (wintitle.f.err || buf == NULL || len == 0)
return 0; return 0;
@ -293,11 +293,10 @@ static size_t get_win_title(char *buf, size_t len)
snprintf(fcnt, ARRLEN(fcnt), "%d", filecnt); snprintf(fcnt, ARRLEN(fcnt), "%d", filecnt);
construct_argv(argv, ARRLEN(argv), wintitle.f.cmd, files[fileidx].path, construct_argv(argv, ARRLEN(argv), wintitle.f.cmd, files[fileidx].path,
fidx, fcnt, w, h, z, NULL); fidx, fcnt, w, h, z, NULL);
pfd = spawn(wintitle.f.cmd, argv, X_READ); if (spawn(&readfd, NULL, argv) > 0) {
if (pfd.readfd >= 0) { if ((n = read(readfd, buf, len-1)) > 0)
if ((n = read(pfd.readfd, buf, len-1)) > 0)
buf[n] = '\0'; buf[n] = '\0';
close(pfd.readfd); close(readfd);
} }
return MAX(0, n); return MAX(0, n);
@ -314,9 +313,7 @@ void close_info(void)
void open_info(void) void open_info(void)
{ {
spawn_t pfd; char *argv[6], w[12] = "", h[12] = "";
char w[12] = "", h[12] = "";
char *argv[6];
char *cmd = mode == MODE_IMAGE ? info.f.cmd : info.ft.cmd; char *cmd = mode == MODE_IMAGE ? info.f.cmd : info.ft.cmd;
bool ferr = mode == MODE_IMAGE ? info.f.err : info.ft.err; bool ferr = mode == MODE_IMAGE ? info.f.err : info.ft.err;
@ -329,12 +326,8 @@ void open_info(void)
} }
construct_argv(argv, ARRLEN(argv), cmd, files[fileidx].name, w, h, construct_argv(argv, ARRLEN(argv), cmd, files[fileidx].name, w, h,
files[fileidx].path, NULL); files[fileidx].path, NULL);
pfd = spawn(cmd, argv, X_READ); if ((info.pid = spawn(&info.fd, NULL, argv)) > 0)
if (pfd.readfd >= 0) { fcntl(info.fd, F_SETFL, O_NONBLOCK);
fcntl(pfd.readfd, F_SETFL, O_NONBLOCK);
info.fd = pfd.readfd;
info.pid = pfd.pid;
}
} }
static void read_info(void) static void read_info(void)
@ -579,13 +572,13 @@ static bool run_key_handler(const char *key, unsigned int mask)
FILE *pfs; FILE *pfs;
bool marked = mode == MODE_THUMB && markcnt > 0; bool marked = mode == MODE_THUMB && markcnt > 0;
bool changed = false; bool changed = false;
int f, i; pid_t pid;
int writefd, f, i;
int fcnt = marked ? markcnt : 1; int fcnt = marked ? markcnt : 1;
char kstr[32]; char kstr[32];
struct stat *oldst, st; struct stat *oldst, st;
XEvent dump; XEvent dump;
char *argv[3]; char *argv[3];
spawn_t pfd;
if (keyhandler.f.err) { if (keyhandler.f.err) {
if (!keyhandler.warned) { if (!keyhandler.warned) {
@ -608,12 +601,11 @@ static bool run_key_handler(const char *key, unsigned int mask)
mask & Mod1Mask ? "M-" : "", mask & Mod1Mask ? "M-" : "",
mask & ShiftMask ? "S-" : "", key); mask & ShiftMask ? "S-" : "", key);
construct_argv(argv, ARRLEN(argv), keyhandler.f.cmd, kstr, NULL); construct_argv(argv, ARRLEN(argv), keyhandler.f.cmd, kstr, NULL);
pfd = spawn(keyhandler.f.cmd, argv, X_WRITE); if ((pid = spawn(NULL, &writefd, argv)) < 0)
if (pfd.writefd < 0)
return false; return false;
if ((pfs = fdopen(pfd.writefd, "w")) == NULL) { if ((pfs = fdopen(writefd, "w")) == NULL) {
error(0, errno, "open pipe"); error(0, errno, "open pipe");
close(pfd.writefd); close(writefd);
return false; return false;
} }
@ -626,7 +618,7 @@ static bool run_key_handler(const char *key, unsigned int mask)
} }
} }
fclose(pfs); fclose(pfs);
while (waitpid(pfd.pid, NULL, 0) == -1 && errno == EINTR); while (waitpid(pid, NULL, 0) == -1 && errno == EINTR);
for (f = i = 0; f < fcnt; i++) { for (f = i = 0; f < fcnt; i++) {
if ((marked && (files[i].flags & FF_MARK)) || (!marked && i == fileidx)) { if ((marked && (files[i].flags & FF_MARK)) || (!marked && i == fileidx)) {

13
nsxiv.h
View File

@ -326,17 +326,6 @@ typedef struct {
int stlen; int stlen;
} r_dir_t; } r_dir_t;
typedef struct {
int readfd;
int writefd;
pid_t pid;
} spawn_t;
enum {
X_READ = (1 << 0),
X_WRITE = (1 << 1)
};
extern const char *progname; extern const char *progname;
void* emalloc(size_t); void* emalloc(size_t);
@ -349,7 +338,7 @@ int r_closedir(r_dir_t*);
char* r_readdir(r_dir_t*, bool); char* r_readdir(r_dir_t*, bool);
int r_mkdir(char*); int r_mkdir(char*);
void construct_argv(char**, unsigned int, ...); void construct_argv(char**, unsigned int, ...);
spawn_t spawn(const char*, char *const [], unsigned int); pid_t spawn(int*, int*, char *const []);
/* window.c */ /* window.c */

111
util.c
View File

@ -21,6 +21,7 @@
#include <assert.h> #include <assert.h>
#include <errno.h> #include <errno.h>
#include <spawn.h>
#include <stdarg.h> #include <stdarg.h>
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
@ -28,6 +29,7 @@
#include <sys/stat.h> #include <sys/stat.h>
#include <unistd.h> #include <unistd.h>
extern char **environ;
const char *progname = "nsxiv"; const char *progname = "nsxiv";
void* emalloc(size_t size) void* emalloc(size_t size)
@ -228,60 +230,65 @@ void construct_argv(char **argv, unsigned int len, ...)
error(EXIT_FAILURE, 0, "argv not NULL terminated"); error(EXIT_FAILURE, 0, "argv not NULL terminated");
} }
spawn_t spawn(const char *cmd, char *const argv[], unsigned int flags) static int mkspawn_pipe(posix_spawn_file_actions_t *fa, const char *cmd, int *pfd, int dupidx)
{ {
pid_t pid; int err;
spawn_t status = { -1, -1, -1 }; if (pipe(pfd) < 0) {
int pfd_read[2] = { -1, -1 }, pfd_write[2] = { -1, -1 };
const bool r = flags & X_READ;
const bool w = flags & X_WRITE;
if (cmd == NULL || argv == NULL || flags == 0)
return status;
if (r && pipe(pfd_read) < 0) {
error(0, errno, "pipe: %s", cmd); error(0, errno, "pipe: %s", cmd);
return status; return -1;
} }
err = posix_spawn_file_actions_adddup2(fa, pfd[dupidx], dupidx);
if (w && pipe(pfd_write) < 0) { err = err ? err : posix_spawn_file_actions_addclose(fa, pfd[0]);
error(0, errno, "pipe: %s", cmd); err = err ? err : posix_spawn_file_actions_addclose(fa, pfd[1]);
if (r) { if (err) {
close(pfd_read[0]); error(0, err, "posix_spawn_file_actions: %s", cmd);
close(pfd_read[1]); close(pfd[0]);
} close(pfd[1]);
return status;
} }
return err ? -1 : 0;
if ((pid = fork()) == 0) { /* in child */ }
if ((r && dup2(pfd_read[1], 1) < 0) || (w && dup2(pfd_write[0], 0) < 0))
error(EXIT_FAILURE, errno, "dup2: %s", cmd); pid_t spawn(int *readfd, int *writefd, char *const argv[])
{
if (r) { pid_t pid = -1;
close(pfd_read[0]); const char *cmd;
close(pfd_read[1]); int err, pfd_read[2], pfd_write[2];
} posix_spawn_file_actions_t fa;
if (w) {
close(pfd_write[0]); assert(argv != NULL && argv[0] != NULL);
close(pfd_write[1]); cmd = argv[0];
}
execv(cmd, argv); if ((err = posix_spawn_file_actions_init(&fa)) != 0) {
error(EXIT_FAILURE, errno, "exec: %s", cmd); error(0, err, "spawn: %s", cmd);
} else if (pid < 0) { /* fork failed */ return pid;
error(0, errno, "fork: %s", cmd); }
if (r)
close(pfd_read[0]); if (readfd != NULL && mkspawn_pipe(&fa, cmd, pfd_read, 1) < 0)
if (w) goto err_destroy_fa;
close(pfd_write[1]); if (writefd != NULL && mkspawn_pipe(&fa, cmd, pfd_write, 0) < 0)
} else { /* in parent */ goto err_close_readfd;
status.pid = pid;
status.readfd = pfd_read[0]; if ((err = posix_spawn(&pid, cmd, &fa, NULL, argv, environ)) != 0) {
status.writefd = pfd_write[1]; error(0, err, "spawn: %s", cmd);
} } else {
if (readfd != NULL)
if (r) *readfd = pfd_read[0];
close(pfd_read[1]); if (writefd != NULL)
if (w) *writefd = pfd_write[1];
close(pfd_write[0]); }
return status;
if (writefd != NULL) {
close(pfd_write[0]);
if (pid < 0)
close(pfd_write[1]);
}
err_close_readfd:
if (readfd != NULL) {
if (pid < 0)
close(pfd_read[0]);
close(pfd_read[1]);
}
err_destroy_fa:
posix_spawn_file_actions_destroy(&fa);
return pid;
} }