Skip to content

Commit

Permalink
refactor(event): change last use of klist to kvec
Browse files Browse the repository at this point in the history
loop->children might have been a linked list because used to be
modified in place while looped over. However the loops that exists
rather schedules events to be processed later, outside of the loop,
so this can not happen anymore.

When a linked list is otherwise useful it is better to use
lib/queue_defs.h which defines an _intrusive_ linked list (i e it
doesn't need to do allocations for list items like klist ).
  • Loading branch information
bfredl committed Sep 28, 2024
1 parent d5f6f61 commit 7616359
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 169 deletions.
144 changes: 0 additions & 144 deletions src/klib/klist.h

This file was deleted.

4 changes: 2 additions & 2 deletions src/nvim/event/loop.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ void loop_init(Loop *loop, void *data)
loop->recursive = 0;
loop->closing = false;
loop->uv.data = loop;
loop->children = kl_init(WatcherPtr);
kv_init(loop->children);
loop->events = multiqueue_new_parent(loop_on_put, loop);
loop->fast_events = multiqueue_new_child(loop->events);
loop->thread_events = multiqueue_new_parent(NULL, NULL);
Expand Down Expand Up @@ -187,7 +187,7 @@ bool loop_close(Loop *loop, bool wait)
multiqueue_free(loop->fast_events);
multiqueue_free(loop->thread_events);
multiqueue_free(loop->events);
kl_destroy(WatcherPtr, loop->children);
kv_destroy(loop->children);
return rv;
}

Expand Down
9 changes: 2 additions & 7 deletions src/nvim/event/loop.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,10 @@
#include <stdbool.h>
#include <uv.h>

#include "klib/klist.h"
#include "klib/kvec.h"
#include "nvim/event/defs.h" // IWYU pragma: keep
#include "nvim/types_defs.h" // IWYU pragma: keep

typedef void *WatcherPtr;

#define NOOP(x)
KLIST_INIT(WatcherPtr, WatcherPtr, NOOP)

struct loop {
uv_loop_t uv;
MultiQueue *events;
Expand All @@ -27,7 +22,7 @@ struct loop {
MultiQueue *fast_events;

// used by process/job-control subsystem
klist_t(WatcherPtr) *children;
kvec_t(Proc *) children;
uv_signal_t children_watcher;
uv_timer_t children_kill_timer;

Expand Down
29 changes: 16 additions & 13 deletions src/nvim/event/proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
#include <signal.h>
#include <uv.h>

#include "klib/klist.h"
#include "nvim/event/libuv_proc.h"
#include "nvim/event/loop.h"
#include "nvim/event/multiqueue.h"
Expand Down Expand Up @@ -123,16 +122,16 @@ int proc_spawn(Proc *proc, bool in, bool out, bool err)
proc->internal_exit_cb = on_proc_exit;
proc->internal_close_cb = decref;
proc->refcount++;
kl_push(WatcherPtr, proc->loop->children, proc);
kv_push(proc->loop->children, proc);
DLOG("new: pid=%d exepath=[%s]", proc->pid, proc_get_exepath(proc));
return 0;
}

void proc_teardown(Loop *loop) FUNC_ATTR_NONNULL_ALL
{
proc_is_tearing_down = true;
kl_iter(WatcherPtr, loop->children, current) {
Proc *proc = (*current)->data;
for (size_t i = 0; i < kv_size(loop->children); i++) {
Proc *proc = kv_A(loop->children, i);
if (proc->detach || proc->type == kProcTypePty) {
// Close handles to process without killing it.
CREATE_EVENT(loop->events, proc_close_handles, proc);
Expand All @@ -143,7 +142,7 @@ void proc_teardown(Loop *loop) FUNC_ATTR_NONNULL_ALL

// Wait until all children exit and all close events are processed.
LOOP_PROCESS_EVENTS_UNTIL(loop, loop->events, -1,
kl_empty(loop->children) && multiqueue_empty(loop->events));
kv_size(loop->children) == 0 && multiqueue_empty(loop->events));
pty_proc_teardown(loop);
}

Expand Down Expand Up @@ -254,8 +253,8 @@ static void children_kill_cb(uv_timer_t *handle)
{
Loop *loop = handle->loop->data;

kl_iter(WatcherPtr, loop->children, current) {
Proc *proc = (*current)->data;
for (size_t i = 0; i < kv_size(loop->children); i++) {
Proc *proc = kv_A(loop->children, i);
bool exited = (proc->status >= 0);
if (exited || !proc->stopped_time) {
continue;
Expand Down Expand Up @@ -294,15 +293,19 @@ static void decref(Proc *proc)
}

Loop *loop = proc->loop;
kliter_t(WatcherPtr) **node = NULL;
kl_iter(WatcherPtr, loop->children, current) {
if ((*current)->data == proc) {
node = current;
size_t i;
for (i = 0; i < kv_size(loop->children); i++) {
Proc *current = kv_A(loop->children, i);
if (current == proc) {
break;
}
}
assert(node);
kl_shift_at(WatcherPtr, loop->children, node);
assert(i < kv_size(loop->children)); // element found
if (i < kv_size(loop->children) - 1) {
memmove(&kv_A(loop->children, i), &kv_A(loop->children, i + 1),
sizeof(&kv_A(loop->children, i)) * (kv_size(loop->children) - (i + 1)));
}
kv_size(loop->children)--;
CREATE_EVENT(proc->events, proc_close_event, proc);
}

Expand Down
5 changes: 2 additions & 3 deletions src/nvim/os/pty_proc_unix.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
#endif

#include "auto/config.h"
#include "klib/klist.h"
#include "nvim/eval/typval.h"
#include "nvim/event/defs.h"
#include "nvim/event/loop.h"
Expand Down Expand Up @@ -387,8 +386,8 @@ static void chld_handler(uv_signal_t *handle, int signum)

Loop *loop = handle->loop->data;

kl_iter(WatcherPtr, loop->children, current) {
Proc *proc = (*current)->data;
for (size_t i = 0; i < kv_size(loop->children); i++) {
Proc *proc = kv_A(loop->children, i);
do {
pid = waitpid(proc->pid, &stat, WNOHANG);
} while (pid < 0 && errno == EINTR);
Expand Down

0 comments on commit 7616359

Please sign in to comment.