diff --git a/.gitignore b/.gitignore index beccf34abe9ee0..3e2e14a510175e 100644 --- a/.gitignore +++ b/.gitignore @@ -112,6 +112,7 @@ /git-mv /git-name-rev /git-notes +/git-odb--daemon /git-p4 /git-pack-redundant /git-pack-objects diff --git a/Makefile b/Makefile index 8cf06d8b367500..dfe285c008ffda 100644 --- a/Makefile +++ b/Makefile @@ -936,6 +936,7 @@ LIB_OBJS += notes.o LIB_OBJS += object-file.o LIB_OBJS += object-name.o LIB_OBJS += object.o +LIB_OBJS += odb-over-ipc.o LIB_OBJS += oid-array.o LIB_OBJS += oidmap.o LIB_OBJS += oidset.o @@ -1119,6 +1120,7 @@ BUILTIN_OBJS += builtin/multi-pack-index.o BUILTIN_OBJS += builtin/mv.o BUILTIN_OBJS += builtin/name-rev.o BUILTIN_OBJS += builtin/notes.o +BUILTIN_OBJS += builtin/odb--daemon.o BUILTIN_OBJS += builtin/pack-objects.o BUILTIN_OBJS += builtin/pack-redundant.o BUILTIN_OBJS += builtin/pack-refs.o diff --git a/builtin.h b/builtin.h index 7554476f90a4bc..0e3080a3abcc5a 100644 --- a/builtin.h +++ b/builtin.h @@ -188,6 +188,7 @@ int cmd_multi_pack_index(int argc, const char **argv, const char *prefix); int cmd_mv(int argc, const char **argv, const char *prefix); int cmd_name_rev(int argc, const char **argv, const char *prefix); int cmd_notes(int argc, const char **argv, const char *prefix); +int cmd_odb__daemon(int argc, const char **argv, const char *prefix); int cmd_pack_objects(int argc, const char **argv, const char *prefix); int cmd_pack_redundant(int argc, const char **argv, const char *prefix); int cmd_patch_id(int argc, const char **argv, const char *prefix); diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index d6b59a98ceddd5..d9b93af01dec6b 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -996,10 +996,24 @@ static int do_handle_client(struct fsmonitor_daemon_state *state, static ipc_server_application_cb handle_client; -static int handle_client(void *data, const char *command, +static int handle_client(void *data, + const char *command, size_t command_len, ipc_server_reply_cb *reply, struct ipc_server_reply_data *reply_data) { + // TODO I did not take time to ensure that `command_len` is + // TODO large enough to do all of the strcmp() and starts_with() + // TODO calculations when I converted the IPC API to take + // TODO `command, command_len` rather than just `command`. + // TODO So some cleanup is needed here. + // TODO + // TODO But we know that FSMonitor always uses regular text + // TODO in both the request and response (no binary messages + // TODO here), so the cleanup is more of the `assert` form. + // TODO + // TODO For example, I should pass `command, command_len` into + // TODO `do_handle_client()` for completeness. + struct fsmonitor_daemon_state *state = data; int result; diff --git a/builtin/odb--daemon.c b/builtin/odb--daemon.c new file mode 100644 index 00000000000000..6b2358f6cad524 --- /dev/null +++ b/builtin/odb--daemon.c @@ -0,0 +1,354 @@ +#include "builtin.h" +#include "config.h" +#include "object-store.h" +#include "oidmap.h" +#include "parse-options.h" +#include "simple-ipc.h" +#include "strbuf.h" +#include "thread-utils.h" +#include "odb-over-ipc.h" + +enum my_mode { + MODE_UNDEFINED = 0, + MODE_RUN, /* run daemon in current process */ + MODE_STOP, /* stop an existing daemon */ +}; + +static const char * const my_usage[] = { + "git odb--daemon --run []", + "git odb--daemon --stop []", + "git odb--daemon TBD...", + NULL +}; + +struct my_args +{ + enum my_mode mode; + int nr_ipc_threads; +}; + +static struct my_args my_args; + +static struct option my_options[] = { + OPT_CMDMODE(0, "run", &my_args.mode, + N_("run the ODB daemon in the foreground"), MODE_RUN), + OPT_CMDMODE(0, "stop", &my_args.mode, + N_("stop an existing ODB daemon"), MODE_STOP), + + OPT_GROUP(N_("Daemon options")), + OPT_INTEGER(0, "ipc-threads", &my_args.nr_ipc_threads, N_("use ipc threads")), + OPT_END() +}; + +#ifndef SUPPORTS_SIMPLE_IPC +int cmd_odb__daemon(int argc, const char **argv, const char *prefix) +{ + if (argc == 2 && !strcmp(argv[1], "-h")) + usage_with_options(my_usage, my_options); + + die(_("odb--daemon not supported on this platform")); +} +#else + +/* + * Technically, we don't need to probe for an existing daemon process + * running on our named pipe / socket, since we could just try to + * create it and let it fail if the pipe/socket is busy. However, + * probing first allows us to give a better error message (especially + * in the case where we disassociate from the terminal and fork into + * the background). + */ +static int is_daemon_listening(void) +{ + return odb_over_ipc__get_state() == IPC_STATE__LISTENING; +} + +struct my_odb_ipc_state +{ + struct ipc_server_data *ipc_state; +}; + +static struct my_odb_ipc_state my_state; + +struct my_oidmap_entry +{ + struct oidmap_entry entry; + struct odb_over_ipc__get_oid__response resp; + char *content; +}; + +static struct oidmap my_oidmap = OIDMAP_INIT; + +static int do_oid_lookup(struct my_oidmap_entry *e, + struct odb_over_ipc__get_oid__request *req) +{ + struct object_info var_oi = OBJECT_INFO_INIT; + int ret; + + var_oi.typep = &e->resp.type; + var_oi.sizep = &e->resp.size; + var_oi.disk_sizep = &e->resp.disk_size; + var_oi.delta_base_oid = &e->resp.delta_base_oid; + /* the client can compute `type_name` from `type`. */ + + if (req->want_content) + var_oi.contentp = (void **)&e->content; + + ret = oid_object_info_extended(the_repository, &req->oid, &var_oi, + req->flags); + if (ret) + return -1; + + // TODO should we create a new fake whence value that we report to + // TODO the client -- something like "ipc" to indicate that the + // TODO client got it from us and therefore doesn't have the in-memory + // TODO cache nor any of the packfile data loaded. The answer to this + // TODO also affects the oi.u.packed fields. + e->resp.whence = var_oi.whence; + + // TODO decide if we care about oi.u.packed + + return 0; +} + +static int odb_ipc_cb__get_oid(struct my_odb_ipc_state *state, + const char *command, size_t command_len, + ipc_server_reply_cb *reply_cb, + struct ipc_server_reply_data *reply_data) +{ + struct odb_over_ipc__get_oid__request *req; + struct my_oidmap_entry *e; + + if (command_len != sizeof(*req)) + BUG("incorrect size for binary data"); + + req = (struct odb_over_ipc__get_oid__request *)command; + + e = oidmap_get(&my_oidmap, &req->oid); +// { +// char hexbuf[200]; +// trace2_printf("get-oid: %s %s", oid_to_hex_r(hexbuf, &req->oid), +// (e ? "found" : "not-found")); +// } + + if (e && req->want_content && !e->content) { + /* + * We have a cached entry from a previous request where + * the client did not want the content, but this client + * does want the content. So repeat the lookup and ammend + * our cache entry. + */ + if (do_oid_lookup(e, req)) + goto fail; + } + + if (!e) { + e = xcalloc(1, sizeof(*e)); + + memcpy(e->resp.key.key, "oid", 4); + oidcpy(&e->resp.oid, &req->oid); + oidclr(&e->resp.delta_base_oid); + + if (do_oid_lookup(e, req)) + goto fail; + + oidcpy(&e->entry.oid, &req->oid); + oidmap_put(&my_oidmap, e); + } + + reply_cb(reply_data, (const char *)&e->resp, sizeof(e->resp)); + if (req->want_content) + reply_cb(reply_data, e->content, e->resp.size); + + return 0; + +fail: + /* + * Send the client an error response to force it to do + * the work itself. + */ + reply_cb(reply_data, "error", 6); + return 0; +} + +/* + * This callback handles IPC requests from clients. We run on an + * arbitrary thread. + * + * We expect `command` to be of the form: + * + * := quit NUL + * | TBC... + */ +static ipc_server_application_cb odb_ipc_cb; + +static int odb_ipc_cb(void *data, + const char *command, size_t command_len, + ipc_server_reply_cb *reply_cb, + struct ipc_server_reply_data *reply_data) +{ + // TODO I did not take time to ensure that `command_len` is + // TODO large enough to do all of the strcmp() and starts_with() + // TODO calculations when I converted the IPC API to take + // TODO `command, command_len` rather than just `command`. + // TODO So some cleanup is needed here. + + struct my_odb_ipc_state *state = data; + int ret; + + assert(state == &my_state); + + if (!strcmp(command, "quit")) { + /* + * A client has requested (over the pipe/socket) that this + * daemon shutdown. + * + * Tell the IPC thread pool to shutdown (which completes the + * `await` in the main thread and causes us to shutdown). + * + * In this callback we are NOT in control of the life of this + * thread, so we cannot directly shutdown here. + */ + return SIMPLE_IPC_QUIT; + } + + if (!strcmp(command, "oid")) { + /* + * A client has requested that we lookup an object from the + * ODB and send it to them. + */ + trace2_region_enter("odb-daemon", "get-oid", NULL); + ret = odb_ipc_cb__get_oid(state, command, command_len, + reply_cb, reply_data); + trace2_region_leave("odb-daemon", "get-oid", NULL); + + return ret; + } + + // TODO respond to other requests from client. + // + // TODO decide how to return an error for unknown commands. + + return 0; +} + +static int launch_ipc_thread_pool(void) +{ + int ret; + const char *path = odb_over_ipc__get_path(); + + struct ipc_server_opts ipc_opts = { + .nr_threads = my_args.nr_ipc_threads, + }; + + ret = ipc_server_run_async(&my_state.ipc_state, + path, &ipc_opts, odb_ipc_cb, &my_state); + + if (ret == -2) /* maybe we lost a startup race */ + error(_("IPC socket/pipe already in use: '%s'"), path); + + else if (ret == -1) + error(_("could not start IPC thread pool on: '%s'"), path); + + return ret; +} + +/* + * Actually run the daemon. + */ +static int do_run_daemon(void) +{ + int ret; + + odb_over_ipc__set_is_daemon(); + + oidmap_init(&my_oidmap, 1024 * 1024); + hashmap_disable_item_counting(&my_oidmap.map); + + // TODO Create mutexes and locks + // + // TODO Load up the packfiles + // + // TODO Consider starting a thread to watch for new/deleted packfiles + // TODO and update the in-memory database. + + ret = launch_ipc_thread_pool(); + if (!ret) + ret = ipc_server_await(my_state.ipc_state); + + // TODO As a precaution, send stop events to our other threads and + // TODO JOIN them. + // + // TODO destroy mutexes and locks. + // + // TODO destroy in-memory databases. + + return ret; +} + +/* + * Try to run the odb--daemon in the foreground of the current process. + */ +static int try_run_daemon(void) +{ + if (is_daemon_listening()) + die("odb--daemon is already running"); + + return do_run_daemon(); +} + +/* + * Acting as a CLIENT. + * + * Send a "quit" command to an existing `git odb--daemon` process + * (if running) and wait for it to shutdown. + * + * The wait here is important for helping the test suite be stable. + */ +static int client_send_stop(void) +{ + struct strbuf answer = STRBUF_INIT; + int ret; + + ret = odb_over_ipc__command("quit", 4, &answer); + + /* The quit command does not return any response data. */ + strbuf_release(&answer); + + if (ret) { + die("could not send stop command to odb--daemon"); + return ret; + } + + trace2_region_enter("odb_client", "polling-for-daemon-exit", NULL); + while (odb_over_ipc__get_state() == IPC_STATE__LISTENING) + sleep_millisec(50); + trace2_region_leave("odb_client", "polling-for-daemon-exit", NULL); + + return 0; +} + +int cmd_odb__daemon(int argc, const char **argv, const char *prefix) +{ + if (argc == 2 && !strcmp(argv[1], "-h")) + usage_with_options(my_usage, my_options); + + git_config(git_default_config, NULL); + + argc = parse_options(argc, argv, prefix, my_options, my_usage, 0); + + if (my_args.nr_ipc_threads < 1) + my_args.nr_ipc_threads = online_cpus(); + + switch (my_args.mode) { + case MODE_RUN: + return !!try_run_daemon(); + + case MODE_STOP: + return !!client_send_stop(); + + default: + die(_("Unhandled command mode")); + } +} +#endif /* SUPPORTS_SIMPLE_IPC */ diff --git a/cache.h b/cache.h index bbf4e601c4beec..858aa2f2dacce3 100644 --- a/cache.h +++ b/cache.h @@ -965,6 +965,8 @@ extern const char *core_fsmonitor; extern int core_apply_sparse_checkout; extern int core_sparse_checkout_cone; +extern int core_use_odb_over_ipc; + /* * Include broken refs in all ref iterations, which will * generally choke dangerous operations rather than letting diff --git a/compat/simple-ipc/ipc-unix-socket.c b/compat/simple-ipc/ipc-unix-socket.c index 49d045010481e0..37d47fd992222e 100644 --- a/compat/simple-ipc/ipc-unix-socket.c +++ b/compat/simple-ipc/ipc-unix-socket.c @@ -150,7 +150,8 @@ void ipc_client_close_connection(struct ipc_client_connection *connection) int ipc_client_send_command_to_connection( struct ipc_client_connection *connection, - const char *message, struct strbuf *answer) + const char *message, size_t message_len, + struct strbuf *answer) { int ret = 0; @@ -158,7 +159,7 @@ int ipc_client_send_command_to_connection( trace2_region_enter("ipc-client", "send-command", NULL); - if (write_packetized_from_buf_no_flush(message, strlen(message), + if (write_packetized_from_buf_no_flush(message, message_len, connection->fd) < 0 || packet_flush_gently(connection->fd) < 0) { ret = error(_("could not send IPC command")); @@ -179,7 +180,8 @@ int ipc_client_send_command_to_connection( int ipc_client_send_command(const char *path, const struct ipc_client_connect_options *options, - const char *message, struct strbuf *answer) + const char *message, size_t message_len, + struct strbuf *answer) { int ret = -1; enum ipc_active_state state; @@ -190,7 +192,9 @@ int ipc_client_send_command(const char *path, if (state != IPC_STATE__LISTENING) return ret; - ret = ipc_client_send_command_to_connection(connection, message, answer); + ret = ipc_client_send_command_to_connection(connection, + message, message_len, + answer); ipc_client_close_connection(connection); @@ -405,7 +409,7 @@ static int do_io_reply_callback(struct ipc_server_reply_data *reply_data, * data), our use of the pkt-line read routines will spew an error * message. * - * Return -1 if the client hung up. + * Return -1 if the client hung up (or we are in a shutdown). * Return 0 if data (possibly incomplete) is ready. */ static int worker_thread__wait_for_io_start( @@ -424,7 +428,7 @@ static int worker_thread__wait_for_io_start( if (result < 0) { if (errno == EINTR) continue; - goto cleanup; + return -1; } if (result == 0) { @@ -441,22 +445,18 @@ static int worker_thread__wait_for_io_start( * client has not started talking yet, just drop it. */ if (in_shutdown) - goto cleanup; + return -1; continue; } if (pollfd[0].revents & POLLHUP) - goto cleanup; + return -1; if (pollfd[0].revents & POLLIN) return 0; - goto cleanup; + return -1; /* should not happen */ } - -cleanup: - close(fd); - return -1; } /* @@ -485,7 +485,7 @@ static int worker_thread__do_io( if (ret >= 0) { ret = worker_thread_data->server_data->application_cb( worker_thread_data->server_data->application_data, - buf.buf, do_io_reply_callback, &reply_data); + buf.buf, buf.len, do_io_reply_callback, &reply_data); packet_flush_gently(reply_data.fd); } @@ -497,7 +497,6 @@ static int worker_thread__do_io( } strbuf_release(&buf); - close(reply_data.fd); return ret; } @@ -548,38 +547,49 @@ static void *worker_thread_proc(void *_worker_thread_data) thread_block_sigpipe(&old_set); +top: for (;;) { fd = worker_thread__wait_for_connection(worker_thread_data); - if (fd == -1) - break; /* in shutdown */ + if (fd == -1) { + /* shutdown already requested */ + goto shutdown; + } - io = worker_thread__wait_for_io_start(worker_thread_data, fd); - if (io == -1) - continue; /* client hung up without sending anything */ + for (;;) { + io = worker_thread__wait_for_io_start(worker_thread_data, fd); + if (io == -1) { + /* client hung up without sending anything */ + close(fd); + goto top; + } - ret = worker_thread__do_io(worker_thread_data, fd); + ret = worker_thread__do_io(worker_thread_data, fd); - if (ret == SIMPLE_IPC_QUIT) { - trace2_data_string("ipc-worker", NULL, "queue_stop_async", - "application_quit"); - /* - * The application layer is telling the ipc-server - * layer to shutdown. - * - * We DO NOT have a response to send to the client. - * - * Queue an async stop (to stop the other threads) and - * allow this worker thread to exit now (no sense waiting - * for the thread-pool shutdown signal). - * - * Other non-idle worker threads are allowed to finish - * responding to their current clients. - */ - ipc_server_stop_async(server_data); - break; + if (ret == SIMPLE_IPC_QUIT) + goto quit; } } +quit: + trace2_data_string("ipc-worker", NULL, "queue_stop_async", + "application_quit"); + /* + * The application layer is telling the ipc-server + * layer to shutdown. + * + * We DO NOT have a response to send to the client. + * + * Queue an async stop (to stop the other threads) and + * allow this worker thread to exit now (no sense waiting + * for the thread-pool shutdown signal). + * + * Other non-idle worker threads are allowed to finish + * responding to their current clients. + */ + ipc_server_stop_async(server_data); + goto shutdown; + +shutdown: trace2_thread_exit(); return NULL; } diff --git a/compat/simple-ipc/ipc-win32.c b/compat/simple-ipc/ipc-win32.c index 8f89c02037e36c..a4d88861689633 100644 --- a/compat/simple-ipc/ipc-win32.c +++ b/compat/simple-ipc/ipc-win32.c @@ -204,7 +204,8 @@ void ipc_client_close_connection(struct ipc_client_connection *connection) int ipc_client_send_command_to_connection( struct ipc_client_connection *connection, - const char *message, struct strbuf *answer) + const char *message, size_t message_len, + struct strbuf *answer) { int ret = 0; @@ -212,7 +213,7 @@ int ipc_client_send_command_to_connection( trace2_region_enter("ipc-client", "send-command", NULL); - if (write_packetized_from_buf_no_flush(message, strlen(message), + if (write_packetized_from_buf_no_flush(message, message_len, connection->fd) < 0 || packet_flush_gently(connection->fd) < 0) { ret = error(_("could not send IPC command")); @@ -235,7 +236,8 @@ int ipc_client_send_command_to_connection( int ipc_client_send_command(const char *path, const struct ipc_client_connect_options *options, - const char *message, struct strbuf *response) + const char *message, size_t message_len, + struct strbuf *response) { int ret = -1; enum ipc_active_state state; @@ -246,7 +248,9 @@ int ipc_client_send_command(const char *path, if (state != IPC_STATE__LISTENING) return ret; - ret = ipc_client_send_command_to_connection(connection, message, response); + ret = ipc_client_send_command_to_connection(connection, + message, message_len, + response); ipc_client_close_connection(connection); @@ -448,23 +452,30 @@ static int do_io(struct ipc_server_thread_data *server_thread_data) return error(_("could not create fd from pipe for '%s'"), server_thread_data->server_data->buf_path.buf); - ret = read_packetized_to_strbuf( - reply_data.fd, &buf, - PACKET_READ_GENTLE_ON_EOF | PACKET_READ_GENTLE_ON_READ_ERROR); - if (ret >= 0) { - ret = server_thread_data->server_data->application_cb( - server_thread_data->server_data->application_data, - buf.buf, do_io_reply_callback, &reply_data); + for (;;) { + strbuf_reset(&buf); + ret = read_packetized_to_strbuf( + reply_data.fd, &buf, + PACKET_READ_GENTLE_ON_EOF | PACKET_READ_GENTLE_ON_READ_ERROR); + if (ret >= 0) { + ret = server_thread_data->server_data->application_cb( + server_thread_data->server_data->application_data, + buf.buf, buf.len, do_io_reply_callback, &reply_data); - packet_flush_gently(reply_data.fd); + packet_flush_gently(reply_data.fd); - FlushFileBuffers((HANDLE)_get_osfhandle((reply_data.fd))); - } - else { - /* - * The client probably disconnected/shutdown before it - * could send a well-formed message. Ignore it. - */ + FlushFileBuffers((HANDLE)_get_osfhandle((reply_data.fd))); + + if (ret == SIMPLE_IPC_QUIT) + break; + } + else { + /* + * The client probably disconnected/shutdown before it + * could send a well-formed message. Ignore it. + */ + break; + } } strbuf_release(&buf); diff --git a/config.c b/config.c index d47d70dc9c643e..4d263e26279643 100644 --- a/config.c +++ b/config.c @@ -1572,6 +1572,11 @@ static int git_default_core_config(const char *var, const char *value, void *cb) return 0; } + if (!strcmp(var, "core.useodboveripc")) { + core_use_odb_over_ipc = git_config_bool(var, value); + return 0; + } + /* Add other config variables here and to Documentation/config.txt. */ return platform_core_config(var, value, cb); } diff --git a/environment.c b/environment.c index 2f27008424a074..c41eea12f2170b 100644 --- a/environment.c +++ b/environment.c @@ -110,6 +110,8 @@ static char *git_namespace; static char *super_prefix; +int core_use_odb_over_ipc; + /* * Repository-local GIT_* environment variables; see cache.h for details. */ diff --git a/fsmonitor-ipc.c b/fsmonitor-ipc.c index b0dc334ff02d43..0b6107bc890769 100644 --- a/fsmonitor-ipc.c +++ b/fsmonitor-ipc.c @@ -63,7 +63,8 @@ int fsmonitor_ipc__send_query(const char *since_token, switch (state) { case IPC_STATE__LISTENING: ret = ipc_client_send_command_to_connection( - connection, since_token, answer); + connection, since_token, strlen(since_token), + answer); ipc_client_close_connection(connection); trace2_data_intmax("fsm_client", NULL, @@ -138,7 +139,9 @@ int fsmonitor_ipc__send_command(const char *command, return -1; } - ret = ipc_client_send_command_to_connection(connection, command, answer); + ret = ipc_client_send_command_to_connection(connection, + command, strlen(command), + answer); ipc_client_close_connection(connection); if (ret == -1) { diff --git a/git.c b/git.c index 239deb9823fcd8..7c6356e7d5d8c3 100644 --- a/git.c +++ b/git.c @@ -556,6 +556,7 @@ static struct cmd_struct commands[] = { { "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE }, { "name-rev", cmd_name_rev, RUN_SETUP }, { "notes", cmd_notes, RUN_SETUP }, + { "odb--daemon", cmd_odb__daemon, RUN_SETUP }, { "pack-objects", cmd_pack_objects, RUN_SETUP }, { "pack-redundant", cmd_pack_redundant, RUN_SETUP | NO_PARSEOPT }, { "pack-refs", cmd_pack_refs, RUN_SETUP }, diff --git a/hackathon-notes-odb-over-ipc.md b/hackathon-notes-odb-over-ipc.md new file mode 100644 index 00000000000000..d1f6b188c81199 --- /dev/null +++ b/hackathon-notes-odb-over-ipc.md @@ -0,0 +1,94 @@ +# Hackathon Results + +(_I'm going to write up my results here in MD and attach it to my notes, rather than creating a separate slide deck that will just get lost._) + +## Title Page + +Hackathon Project: Git ODB over IPC +@jeffhostetler, Git-Client Team +April 12-16, 2021 + +https://github.com/jeffhostetler/git/pull/12 + +Based upon a suggestion from @derrickstolee + +## Questions + +1. Can we improve the performance of local Git commands with a local object database (ODB) daemon? + - All object lookups are modified to ask the daemon rather than directly accessing the disk. + - The daemon responds to client object requests, accesses the disk, and provides advanced cacheing. +2. Can we build upon the "Simple IPC" layer and thread pool mechanism created for FSMonitor? + +## ODB Background + +Individual objects (commits, trees, blobs) are stored in a highly-compacted and storage-efficient set of files on disk: + +1. Shared packfiles (`*.pack`, `*.idx`, and `*.midx`) files in one or more Git Alternates. +2. Private packfiles (`*.pack`, `*.idx`, and `*.midx`) in `.git/objects/pack/`. +3. Loose objects (`.git/objects/XX/*`) + +Packfiles use compression and delta-chains to reduce disk space (and network downloads), but it can be expensive to find and regenerate an individual object. + +## Opportunities for Perf + +Each Git command needs to access the ODB to lookup individual objects. Conceptually, there are four steps required: + +1. Scan the various shared and private packfile directories for `*.idx` and `*.midx` indexes and construct a list of in-memory dictionaries of object locations within packfiles. (_While this list may be lazily created, it is still a startup cost in each Git process._) +2. When fetching an individual object, `mmap` packfile any fragments necessary to reconstruct the object. (_These fragments are mapped with generous address rounding on both ends, so we usually have good overlap with future object requests. However, `mmap` isn't free and we do have to pay for page-faults. And again, this memory mapping has to happen for each Git process._) +3. Within a packfile, an object is `zlib` compressed and it must be zlib-inflated to regenerate it. This is expensive. (_When I was working on parallel checkout, I noticed that a significant percentage of the time was spent in zlib._) +4. Within a packfile, an object may be part of a delta-chain and may require a sequence of the [2] and [3] steps to regenerate the "data-base" before the current object may be regenerated. + +Each of these steps gives us a "perf opportunity". + +1. An ODB daemon could manage (preserve between clients) the list of index dictionaries. (_Extra credit if it also watches the file system for created/deleted .idx files and automatically adapts._) +2. An ODB daemon could similarly manage the packfile memory mappings. +3. An ODB daemon could cache regenerated objects and respond without touching the disk, re-inflating, or re-de-delta-ing it. + +## The State of the HACK + +1. Create `git odb--daemon` + 1.1. This is a long-running thread pool-based service/daemon modeled on my FSMonitor daemon. + 1.2. It speaks IPC over Windows Named Pipes or Unix domain sockets. + 1.3. It caches all fetched objects and can re-serve them without touching the disk. + +2. IPC upgrades + 2.1. Keep Alive: I upgraded the "Simple IPC" code used in FSMonitor to support multiple requests/responses on the same connection. (FSMonitor only needed a simple single request/response model.) + 2.2. Binary Mode: I also upgraded it to allow binary transfers. + +3. In all Git commands: I intercept all object lookups and route to the ODB daemon instead of directly touching the disk. + +## Limitations of the HACK + +_This is a hackathon project, so I did cut some corners._ + +1. On the client side, I intercepted all object lookups and route them to the ODB daemon and bypassed any client-side cacheing available. A proper version would work with the existing in-process cache. + +2. On the daemon side, I added a simple `oidmap` cache to remember and re-serve previously fetched objects. No attempt was made to manage this cache -- it will just consume all memory in the daemon process. + +3. On the daemon side, I do have a thread pool to service concurrent clients, but I didn't bother to make the `oidmap` cache thread safe or deal with any thread issues with in the existing ODB code. + +4. The daemon is fairly limited and just responds to object requests. There is no packfile management or pre-loading. + +## Demo Results (using git.git) + +| | r1 | r2 | r3 | r4 | **avg** | r1 | r2 | r3 | r4 | **avg** | +/- | +| --- | -- | -- | -- | -- | --- | -- | -- | -- | -- | --- | --- | +| [1] | 0.07 | 0.07 | 0.07 | 0.07 | **0.07** | 0.08 | 0.08 | 0.08 | 0.10 | **0.085** | +0.015 | +| [2] | 0.07 | 0.07 | 0.07 | 0.07 | **0.07** | 0.08 | 0.07 | 0.07 | 0.07 | **0.073** | +0.003 | +| [3] | 0.05 | 0.04 | 0.04 | 0.04 | **0.043** | 0.04 | 0.05 | 0.05 | 0.05 | **0.048** | +0.005 | +| [4] | 0.06 | 0.06 | 0.05 | 0.06 | **0.058** | 0.07 | 0.07 | 0.06 | 0.06 | **0.065** | +0.007 | +| [5] | 7.93 | 7.85 | 7.92 | 8.07 | **7.943** | 7.88 | 7.94 | 8.06 | 8.56 | **8.110** | +0.167 | +| [6] | 9.84 | 9.80 | 9.70 | 10.97 | **10.078** | 9.89 | 9.69 | 9.82 | 9.70 | **9.775** | -0.303 | + +[1] `git status >/dev/null` +[2] `git diff HEAD~1 >/dev/null` +[3] `git log -200 >/dev/null` +[4] `git rev-list --objects HEAD^{tree} >/dev/null` +[5] `git rev-list --objects HEAD >/dev/null` +[6] `git rev-list --objects --all >/dev/null` + +## Results + +Perf-wise, the before and after numbers are about equal. More testing is needed to see where time is being spent and where we can optimize, but this (relatively stupid) daemon demonstrates that an IPC-based solution is possible. + +This lets us think about creating a smarter daemon to manage the ODB and access to it. diff --git a/object-file.c b/object-file.c index 624af408cdcd2a..1140111013d211 100644 --- a/object-file.c +++ b/object-file.c @@ -32,6 +32,7 @@ #include "packfile.h" #include "object-store.h" #include "promisor-remote.h" +#include "odb-over-ipc.h" /* The maximum size for an object header. */ #define MAX_HEADER_LEN 32 @@ -1566,9 +1567,18 @@ int oid_object_info_extended(struct repository *r, const struct object_id *oid, struct object_info *oi, unsigned flags) { int ret; + + if (!odb_over_ipc__get_oid(r, oid, oi, flags)) + return 0; + + trace2_region_enter("oid", "object", r); + obj_read_lock(); ret = do_oid_object_info_extended(r, oid, oi, flags); obj_read_unlock(); + + trace2_region_leave("oid", "object", r); + return ret; } diff --git a/odb-over-ipc.c b/odb-over-ipc.c new file mode 100644 index 00000000000000..f9788920c49b81 --- /dev/null +++ b/odb-over-ipc.c @@ -0,0 +1,205 @@ +#include "cache.h" +#include "object.h" +#include "object-store.h" +#include "simple-ipc.h" +#include "odb-over-ipc.h" + +int odb_over_ipc__is_supported(void) +{ +#ifdef SUPPORTS_SIMPLE_IPC + return 1; +#else + return 0; +#endif +} + +#ifdef SUPPORTS_SIMPLE_IPC +/* + * We claim "/odb-over-ipc" as the name of the Unix Domain Socket + * that we will use on Unix. And something based on this unique string + * in the Named Pipe File System on Windows. So we don't need a command + * line argument for this. + */ +GIT_PATH_FUNC(odb_over_ipc__get_path, "odb-over-ipc"); + +static int is_daemon = 0; + +void odb_over_ipc__set_is_daemon(void) +{ + is_daemon = 1; +} + +enum ipc_active_state odb_over_ipc__get_state(void) +{ + return ipc_get_active_state(odb_over_ipc__get_path()); +} + +// TODO This is a hackathon project, so I'm not going to worry about +// TODO ensuring that full threading works right now. That is, I'm +// TODO NOT going to give each thread its own connection to the server +// TODO and I'm NOT going to install locking to let concurrent threads +// TODO properly share a single connection. +// TODO +// TODO We already know that the ODB has limited thread-safety, so I'm +// TODO going to rely on our callers to already behave themselves. +// +static struct ipc_client_connection *my_ipc_connection; +static int my_ipc_available = -1; + +// TOOD We need someone to call this to close our connection after we +// TODO have finished with the ODB. Yes, it will be implicitly closed +// TODO when the foreground Git client process exits, but we are +// TODO holding a connection and thread in the `git odb--daemon` open +// TODO and should try to release it quickly. +// +void odb_over_ipc__shutdown_keepalive_connection(void) +{ + if (my_ipc_connection) { + ipc_client_close_connection(my_ipc_connection); + my_ipc_connection = NULL; + } + + /* + * Assume that we shutdown a fully functioning connection and + * could reconnect again if desired. Our caller can reset this + * assumption, for example when it gets an error. + */ + my_ipc_available = 1; +} + +int odb_over_ipc__command(const char *command, size_t command_len, + struct strbuf *answer) +{ + int ret; + + if (my_ipc_available == -1) { + enum ipc_active_state state; + struct ipc_client_connect_options options + = IPC_CLIENT_CONNECT_OPTIONS_INIT; + + options.wait_if_busy = 1; + options.wait_if_not_found = 0; + + state = ipc_client_try_connect(odb_over_ipc__get_path(), &options, + &my_ipc_connection); + if (state != IPC_STATE__LISTENING) { + // error("odb--daemon is not running"); + my_ipc_available = 0; + return -1; + } + + my_ipc_available = 1; + } + if (!my_ipc_available) + return -1; + + strbuf_reset(answer); + + ret = ipc_client_send_command_to_connection(my_ipc_connection, + command, command_len, + answer); + + if (ret == -1) { + error("could not send '%s' command to odb--daemon", command); + odb_over_ipc__shutdown_keepalive_connection(); + my_ipc_available = 0; + return -1; + } + + return 0; +} + +/* + * When we request an object from the daemon over IPC, the response + * contains both the response-header and the content of the object in + * one buffer. We want to pre-alloc the strbuf-buffer big enough to + * avoid multiple realloc's when we are receiving large blobs. + * + * IPC uses pkt-line and will handle the chunking and reassembly, so + * we are not limited to LARGE_PACKET_DATA_MAX buffers. + */ +#define LARGE_ANSWER (64 * 1024) + +int odb_over_ipc__get_oid(struct repository *r, const struct object_id *oid, + struct object_info *oi, unsigned flags) +{ + struct odb_over_ipc__get_oid__request req; + struct odb_over_ipc__get_oid__response *resp; + + struct strbuf answer = STRBUF_INIT; + const char *content; + ssize_t content_len; + int ret; + + if (is_daemon) + return -1; + + if (!core_use_odb_over_ipc) + return -1; + + if (r != the_repository) // TODO not dealing with this + return -1; + + memset(&req, 0, sizeof(req)); + memcpy(req.key.key, "oid", 4); + oidcpy(&req.oid, oid); + req.flags = flags; + req.want_content = (oi && oi->contentp); + + strbuf_init(&answer, LARGE_ANSWER); + + ret = odb_over_ipc__command((const char *)&req, sizeof(req), &answer); + if (ret) + return ret; + + if (!strncmp(answer.buf, "error", 5)) { + trace2_printf("odb-over-ipc: failed for '%s'", oid_to_hex(oid)); + return -1; + } + + if (!oi) { + /* + * The caller doesn't care about the object itself; + * just whether it exists?? + */ + goto done; + } + + if (answer.len < sizeof(*resp)) + BUG("incorrect size for binary data"); + resp = (struct odb_over_ipc__get_oid__response *)answer.buf; + + content = answer.buf + sizeof(*resp); + content_len = answer.len - sizeof(*resp); + + if (!oideq(&resp->oid, oid)) { + // TODO Think about the _LOOKUP_REPLACE code here + BUG("received different OID"); + } + + if (oi->typep) + *(oi->typep) = resp->type; + if (oi->type_name) + strbuf_addstr(oi->type_name, type_name(resp->type)); + if (oi->sizep) + *(oi->sizep) = resp->size; + if (oi->disk_sizep) + *(oi->disk_sizep) = resp->disk_size; + if (oi->delta_base_oid) + oidcpy(oi->delta_base_oid, &resp->delta_base_oid); + oi->whence = resp->whence; + if (oi->contentp) { + if (content_len != resp->size) + BUG("observed content length does not match size"); + *oi->contentp = xmemdupz(content, content_len); + } + + // TODO The server does not send the contents of oi.u.packed. + // TODO Do we care? + +done: + strbuf_release(&answer); + return ret; +} + +#endif /* SUPPORTS_SIMPLE_IPC */ diff --git a/odb-over-ipc.h b/odb-over-ipc.h new file mode 100644 index 00000000000000..98524dd1c5015a --- /dev/null +++ b/odb-over-ipc.h @@ -0,0 +1,99 @@ +#ifndef ODB_OVER_IPC_H +#define ODB_OVER_IPC_H + +/* + * Returns true if Simple IPC is supported on this platform. This symbol + * must always be visible and outside of the ifdef. + */ +int odb_over_ipc__is_supported(void); + +#include "simple-ipc.h" + +#ifdef SUPPORTS_SIMPLE_IPC + +/* + * Returns the pathname to the IPC named pipe or Unix domain socket + * where a `git odb--daemon` process will listen. + * + * TODO Technically, this is a per repo value, since it needs to + * TODO look at the ODB (both the `/objects` directory and + * TODO ODB alternates). So we should share a daemon between multiple + * TODO worktrees. Verify this. + */ +const char *odb_over_ipc__get_path(void); + +/* + * Try to determine whether there is a `git odb--daemon` process + * listening on the official IPC named pipe / socket for the + * current repo. + */ +enum ipc_active_state odb_over_ipc__get_state(void); + +/* + * Connect to an existing `git odb--daemon` process, send a command, + * and receive a response. If no daemon is running, this DOES NOT try + * to start one. + * + * TODO If we can trust the code that creates/deletes packfiles, we + * TODO might consider adding a command here to let that process tell + * TODO the daemon to update the list of cached packfiles. + * + * TODO For simplicit during prototyping I am NOT going to + * TODO auto-start one. Revisit this later. + */ +int odb_over_ipc__command(const char *command, size_t command_len, + struct strbuf *answer); + +struct odb_over_ipc__key +{ + char key[16]; +}; + +struct odb_over_ipc__get_oid__request +{ + struct odb_over_ipc__key key; + struct object_id oid; + unsigned flags; + unsigned want_content:1; +}; + +struct odb_over_ipc__get_oid__response +{ + struct odb_over_ipc__key key; + struct object_id oid; + struct object_id delta_base_oid; + off_t disk_size; + unsigned long size; + unsigned whence; /* see struct object_info */ + enum object_type type; +}; + +/* + * Connect to an existing `git odb--daemon` process and ask it for + * an object. This is intended to be inserted into the client + * near `oid_object_info_extended()`. + * + * Returns non-zero when the caller should use the traditional + * method. + */ +struct object_info; +struct repository; + +int odb_over_ipc__get_oid(struct repository *r, const struct object_id *oid, + struct object_info *oi, unsigned flags); + +/* + * Explicitly shutdown IPC connection to the `git odb--daemon` process. + * The connection is implicitly created upon the first request and we + * use a keep-alive model to re-use it for subsequent requests. + */ +void odb_over_ipc__shutdown_keepalive_connection(void); + +/* + * Insurance to protect the daemon from calling ODB code and accidentally + * falling into the client-side code and trying to connect to itself. + */ +void odb_over_ipc__set_is_daemon(void); + +#endif /* SUPPORTS_SIMPLE_IPC */ +#endif /* ODB_OVER_IPC_H */ diff --git a/simple-ipc.h b/simple-ipc.h index dc3606e30bd619..c4d5225b41c2a4 100644 --- a/simple-ipc.h +++ b/simple-ipc.h @@ -111,7 +111,8 @@ void ipc_client_close_connection(struct ipc_client_connection *connection); */ int ipc_client_send_command_to_connection( struct ipc_client_connection *connection, - const char *message, struct strbuf *answer); + const char *message, size_t message_len, + struct strbuf *answer); /* * Used by the client to synchronously connect and send and receive a @@ -123,7 +124,8 @@ int ipc_client_send_command_to_connection( */ int ipc_client_send_command(const char *path, const struct ipc_client_connect_options *options, - const char *message, struct strbuf *answer); + const char *message, size_t message_len, + struct strbuf *answer); /* * Simple IPC Server Side API. @@ -148,6 +150,7 @@ typedef int (ipc_server_reply_cb)(struct ipc_server_reply_data *, */ typedef int (ipc_server_application_cb)(void *application_data, const char *request, + size_t request_len, ipc_server_reply_cb *reply_cb, struct ipc_server_reply_data *reply_data); diff --git a/t/helper/test-simple-ipc.c b/t/helper/test-simple-ipc.c index 42040ef81b1e84..fb1638e179d5e4 100644 --- a/t/helper/test-simple-ipc.c +++ b/t/helper/test-simple-ipc.c @@ -112,7 +112,7 @@ static int app__slow_command(ipc_server_reply_cb *reply_cb, /* * The client sent a command followed by a (possibly very) large buffer. */ -static int app__sendbytes_command(const char *received, +static int app__sendbytes_command(const char *received, size_t received_len, ipc_server_reply_cb *reply_cb, struct ipc_server_reply_data *reply_data) { @@ -123,6 +123,12 @@ static int app__sendbytes_command(const char *received, int errs = 0; int ret; + // TODO I did not take time to ensure that `received_len` is + // TODO large enough to do all of the skip_prefix() + // TODO calculations when I converted the IPC API to take + // TODO `received, received_len` rather than just `received`. + // TODO So some cleanup is needed here. + if (skip_prefix(received, "sendbytes ", &p)) len_ballast = strlen(p); @@ -160,10 +166,16 @@ static ipc_server_application_cb test_app_cb; * by this application. */ static int test_app_cb(void *application_data, - const char *command, + const char *command, size_t command_len, ipc_server_reply_cb *reply_cb, struct ipc_server_reply_data *reply_data) { + // TODO I did not take time to ensure that `command_len` is + // TODO large enough to do all of the strcmp() and starts_with() + // TODO calculations when I converted the IPC API to take + // TODO `command, command_len` rather than just `command`. + // TODO So some cleanup is needed here. + /* * Verify that we received the application-data that we passed * when we started the ipc-server. (We have several layers of @@ -208,7 +220,8 @@ static int test_app_cb(void *application_data, return app__slow_command(reply_cb, reply_data); if (starts_with(command, "sendbytes ")) - return app__sendbytes_command(command, reply_cb, reply_data); + return app__sendbytes_command(command, command_len, + reply_cb, reply_data); return app__unhandled_command(command, reply_cb, reply_data); } @@ -488,7 +501,9 @@ static int client__send_ipc(void) options.wait_if_busy = 1; options.wait_if_not_found = 0; - if (!ipc_client_send_command(cl_args.path, &options, command, &buf)) { + if (!ipc_client_send_command(cl_args.path, &options, + command, strlen(command), + &buf)) { if (buf.len) { printf("%s\n", buf.buf); fflush(stdout); @@ -556,7 +571,9 @@ static int do_sendbytes(int bytecount, char byte, const char *path, strbuf_addstr(&buf_send, "sendbytes "); strbuf_addchars(&buf_send, byte, bytecount); - if (!ipc_client_send_command(path, options, buf_send.buf, &buf_resp)) { + if (!ipc_client_send_command(path, options, + buf_send.buf, buf_send.len, + &buf_resp)) { strbuf_rtrim(&buf_resp); printf("sent:%c%08d %s\n", byte, bytecount, buf_resp.buf); fflush(stdout); @@ -676,6 +693,50 @@ static int client__multiple(void) return (sum_join_errors + sum_thread_errors) ? 1 : 0; } +/* + * Send a series of commands over a single connection keepalive-style. + */ +static int client__keepalive(void) +{ + struct ipc_client_connection *connection = NULL; + struct ipc_client_connect_options options + = IPC_CLIENT_CONNECT_OPTIONS_INIT; + struct strbuf answer = STRBUF_INIT; + int ret = 0; + int k; + enum ipc_active_state state; + + strbuf_reset(&answer); + + options.wait_if_busy = 1; + options.wait_if_not_found = 0; + options.uds_disallow_chdir = 0; + + state = ipc_client_try_connect(cl_args.path, &options, &connection); + if (state != IPC_STATE__LISTENING) + return error("failed to connect to '%s'", cl_args.path); + + for (k = 0; k < 10; k++) { + ret = ipc_client_send_command_to_connection(connection, + "ping", 4, + &answer); + if (ret) { + error("failed to send ping[%d]", k); + goto cleanup; + } + if (answer.len) { + printf("%s\n", answer.buf); + fflush(stdout); + } + } + +cleanup: + ipc_client_close_connection(connection); + strbuf_release(&answer); + + return ret; +} + int cmd__simple_ipc(int argc, const char **argv) { const char * const simple_ipc_usage[] = { @@ -686,6 +747,7 @@ int cmd__simple_ipc(int argc, const char **argv) N_("test-helper simple-ipc send [] []"), N_("test-helper simple-ipc sendbytes [] [] []"), N_("test-helper simple-ipc multiple [] [] [] []"), + N_("test-helper simple-ipc keepalive []"), NULL }; @@ -782,6 +844,12 @@ int cmd__simple_ipc(int argc, const char **argv) return !!client__multiple(); } + if (!strcmp(cl_args.subcommand, "keepalive")) { + if (client__probe_server()) + return 1; + return !!client__keepalive(); + } + die("Unhandled subcommand: '%s'", cl_args.subcommand); } #endif diff --git a/t/perf/p7100-odb-daemon.sh b/t/perf/p7100-odb-daemon.sh new file mode 100755 index 00000000000000..80abc4dfb7b53f --- /dev/null +++ b/t/perf/p7100-odb-daemon.sh @@ -0,0 +1,104 @@ +#!/bin/sh + +test_description="Test git odb--daemon" + +. ./perf-lib.sh + +test_perf_large_repo +test_checkout_worktree + +test_odb_daemon_suite() { + test_perf "status ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE status >/dev/null + ' + test_perf "status ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE status >/dev/null + ' + test_perf "status ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE status >/dev/null + ' + test_perf "status ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE status >/dev/null + ' + + test_perf "diff ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE diff HEAD~1 >/dev/null + ' + test_perf "diff ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE diff HEAD~1 >/dev/null + ' + test_perf "diff ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE diff HEAD~1 >/dev/null + ' + test_perf "diff ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE diff HEAD~1 >/dev/null + ' + + test_perf "log -200 ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE log -200 >/dev/null + ' + test_perf "log -200 ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE log -200 >/dev/null + ' + test_perf "log -200 ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE log -200 >/dev/null + ' + test_perf "log -200 ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE log -200 >/dev/null + ' + + test_perf "rev-list HEAD^{tree} ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE rev-list --objects HEAD^{tree} >/dev/null + ' + test_perf "rev-list HEAD^{tree} ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE rev-list --objects HEAD^{tree} >/dev/null + ' + test_perf "rev-list HEAD^{tree} ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE rev-list --objects HEAD^{tree} >/dev/null + ' + test_perf "rev-list HEAD^{tree} ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE rev-list --objects HEAD^{tree} >/dev/null + ' + + test_perf "rev-list HEAD ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE rev-list --objects HEAD >/dev/null + ' + test_perf "rev-list HEAD ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE rev-list --objects HEAD >/dev/null + ' + test_perf "rev-list HEAD ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE rev-list --objects HEAD >/dev/null + ' + test_perf "rev-list HEAD ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE rev-list --objects HEAD >/dev/null + ' + + test_perf "rev-list --all ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE rev-list --objects --all >/dev/null + ' + test_perf "rev-list --all ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE rev-list --objects --all >/dev/null + ' + test_perf "rev-list --all ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE rev-list --objects --all >/dev/null + ' + test_perf "rev-list --all ($ENABLE)" ' + git -c core.useodboveripc=$ENABLE rev-list --objects --all >/dev/null + ' +} + +ENABLE=0 +test_odb_daemon_suite + +test_expect_success "Start odb--daemon" ' + (git odb--daemon --run &) +' + +ENABLE=1 +test_odb_daemon_suite + +test_expect_success "Stop odb--daemon" ' + git odb--daemon --stop +' + +test_done diff --git a/t/t0052-simple-ipc.sh b/t/t0052-simple-ipc.sh index ff98be31a51b36..9737e897fc5400 100755 --- a/t/t0052-simple-ipc.sh +++ b/t/t0052-simple-ipc.sh @@ -113,6 +113,15 @@ test_expect_success 'stress test threads' ' test_cmp expect_a actual_a ' +# Test keepalive feature. Have client open a single connection +# and exchange a series of requests/responses over it. +# +test_expect_success 'keepalive' ' + test-tool simple-ipc keepalive >actual && + grep "pong" actual >actual_pong && + test_line_count = 10 actual_pong +' + test_expect_success 'stop-daemon works' ' test-tool simple-ipc stop-daemon && test_must_fail test-tool simple-ipc is-active &&