From 1db1f9a3960cacd10c76da0613a882caaf18a02b Mon Sep 17 00:00:00 2001 From: Sean McBride Date: Wed, 15 Jun 2022 15:17:07 -0400 Subject: [PATCH] refactor: Address Emil feedback --- runtime/include/http_session.h | 47 +++++++++++++--------- runtime/include/sandbox_perf_log.h | 2 +- runtime/include/sandbox_set_as_allocated.h | 1 + runtime/include/sandbox_set_as_returned.h | 1 - runtime/include/sandbox_set_as_runnable.h | 1 + runtime/include/sandbox_types.h | 3 +- runtime/include/tcp_session.h | 12 +++--- runtime/include/wasm_stack.h | 3 +- runtime/src/current_sandbox.c | 4 +- runtime/src/listener_thread.c | 19 +++++---- runtime/src/sandbox.c | 3 +- 11 files changed, 52 insertions(+), 44 deletions(-) diff --git a/runtime/include/http_session.h b/runtime/include/http_session.h index e6e4e7d..743b7de 100644 --- a/runtime/include/http_session.h +++ b/runtime/include/http_session.h @@ -54,6 +54,7 @@ struct http_session { size_t response_buffer_written; struct tenant *tenant; /* Backlink required when read blocks on listener core */ uint64_t request_arrival_timestamp; + uint64_t response_sent_timestamp; }; /** @@ -196,7 +197,7 @@ http_session_close(struct http_session *session) * Writes an HTTP header to the client * @param client_socket - the client * @param on_eagain - cb to execute when client socket returns EAGAIN. If NULL, error out - * @returns 0 on success, -1 on error, -2 unused, -3 on eagain + * @returns 0 on success, -errno on error */ static inline int http_session_send_response_header(struct http_session *session, void_star_cb on_eagain) @@ -223,7 +224,7 @@ http_session_send_response_header(struct http_session *session, void_star_cb on_ * Writes an HTTP body to the client * @param client_socket - the client * @param on_eagain - cb to execute when client socket returns EAGAIN. If NULL, error out - * @returns 0 on success, -1 on error, -2 unused, -3 on eagain + * @returns 0 on success, -errno on error */ static inline int http_session_send_response_body(struct http_session *session, void_star_cb on_eagain) @@ -346,7 +347,7 @@ http_session_log_malformed_request(struct http_session *session) /** * Receive and Parse the Request for the current sandbox - * @return 0 if message parsing complete, -1 on error, -2 if buffers run out of space, -3 EAGAIN + * @return 0 if message parsing complete, -1 on error, -ENOMEM if buffers run out of space, -3 EAGAIN if would block */ static inline int http_session_receive_request(struct http_session *session, void_star_cb on_eagain) @@ -380,10 +381,13 @@ http_session_receive_request(struct http_session *session, void_star_cb on_eagai (char *)&session->request_buffer.buffer[session->request_buffer.length], session->request_buffer.capacity - session->request_buffer.length, on_eagain, session); - if (bytes_received == -3) goto err_eagain; - if (bytes_received == -1) goto err; + if (unlikely(bytes_received == -EAGAIN)) + goto err_eagain; + else if (unlikely(bytes_received < 0)) + goto err; /* If we received an EOF before we were able to parse a complete HTTP message, request is malformed */ - if (bytes_received == 0 && !session->http_request.message_end) goto err; + else if (unlikely(bytes_received == 0 && !session->http_request.message_end)) + goto err; assert(bytes_received > 0); assert(session->request_buffer.length < session->request_buffer.capacity); @@ -403,11 +407,11 @@ http_session_receive_request(struct http_session *session, void_star_cb on_eagai done: return rc; err_eagain: - rc = -3; + rc = -EAGAIN; goto done; err_nobufs: http_session_log_malformed_request(session); - rc = -2; + rc = -ENOMEM; goto done; err: http_session_log_malformed_request(session); @@ -449,23 +453,26 @@ static inline void http_session_send_response(struct http_session *session, void_star_cb on_eagain) { int rc = http_session_send_response_header(session, on_eagain); - if (unlikely(rc == -3)) { - /* session blocked and registered to epoll so continue to next handle */ - return; - } else if (unlikely(rc == -1)) { - http_session_close(session); - return; + /* session blocked and registered to epoll so continue to next handle */ + if (unlikely(rc == -EAGAIN)) { + goto DONE; + } else if (unlikely(rc < 0)) { + goto ERR; } rc = http_session_send_response_body(session, on_eagain); - if (unlikely(rc == -3)) { - /* session blocked and registered to epoll so continue to next handle */ - return; - } else if (unlikely(rc == -1)) { - http_session_close(session); - return; + /* session blocked and registered to epoll so continue to next handle */ + if (unlikely(rc == -EAGAIN)) { + goto DONE; + } else if (unlikely(rc < 0)) { + goto ERR; } + session->response_sent_timestamp = __getcycles(); + +DONE: + return; +ERR: http_session_close(session); http_session_free(session); } diff --git a/runtime/include/sandbox_perf_log.h b/runtime/include/sandbox_perf_log.h index e76261a..6becea3 100644 --- a/runtime/include/sandbox_perf_log.h +++ b/runtime/include/sandbox_perf_log.h @@ -29,7 +29,7 @@ sandbox_perf_log_print_entry(struct sandbox *sandbox) /* If the log was not defined by an environment variable, early out */ if (sandbox_perf_log == NULL) return; - uint64_t queued_duration = sandbox->timestamp_of.allocation - sandbox->timestamp_of.request_arrival; + uint64_t queued_duration = sandbox->timestamp_of.dispatched - sandbox->timestamp_of.allocation; /* * Assumption: A sandbox is never able to free pages. If linear memory management diff --git a/runtime/include/sandbox_set_as_allocated.h b/runtime/include/sandbox_set_as_allocated.h index f77b215..89a5fe7 100644 --- a/runtime/include/sandbox_set_as_allocated.h +++ b/runtime/include/sandbox_set_as_allocated.h @@ -24,6 +24,7 @@ sandbox_set_as_allocated(struct sandbox *sandbox) /* State Change Bookkeeping */ assert(now > sandbox->timestamp_of.last_state_change); + sandbox->timestamp_of.allocation = now; sandbox->timestamp_of.last_state_change = now; sandbox_state_history_init(&sandbox->state_history); sandbox_state_history_append(&sandbox->state_history, SANDBOX_ALLOCATED); diff --git a/runtime/include/sandbox_set_as_returned.h b/runtime/include/sandbox_set_as_returned.h index 3a20fc9..88cc472 100644 --- a/runtime/include/sandbox_set_as_returned.h +++ b/runtime/include/sandbox_set_as_returned.h @@ -31,7 +31,6 @@ sandbox_set_as_returned(struct sandbox *sandbox, sandbox_state_t last_state) switch (last_state) { case SANDBOX_RUNNING_SYS: { - sandbox->timestamp_of.response = now; local_runqueue_delete(sandbox); sandbox_free_linear_memory(sandbox); break; diff --git a/runtime/include/sandbox_set_as_runnable.h b/runtime/include/sandbox_set_as_runnable.h index 8e28875..6b404d5 100644 --- a/runtime/include/sandbox_set_as_runnable.h +++ b/runtime/include/sandbox_set_as_runnable.h @@ -30,6 +30,7 @@ sandbox_set_as_runnable(struct sandbox *sandbox, sandbox_state_t last_state) switch (last_state) { case SANDBOX_INITIALIZED: { + sandbox->timestamp_of.dispatched = now; local_runqueue_add(sandbox); break; } diff --git a/runtime/include/sandbox_types.h b/runtime/include/sandbox_types.h index f9f2ca1..796c4b3 100644 --- a/runtime/include/sandbox_types.h +++ b/runtime/include/sandbox_types.h @@ -21,9 +21,8 @@ struct sandbox_timestamps { uint64_t last_state_change; /* Used for bookkeeping of actual execution time */ - uint64_t request_arrival; /* Timestamp when request is received */ uint64_t allocation; /* Timestamp when sandbox is allocated */ - uint64_t response; /* Timestamp when response is sent */ + uint64_t dispatched; /* Timestamp when a sandbox is first added to a worker's runqueue */ uint64_t completion; /* Timestamp when sandbox runs to completion */ #ifdef LOG_SANDBOX_MEMORY_PROFILE uint32_t page_allocations[SANDBOX_PAGE_ALLOCATION_TIMESTAMP_COUNT]; diff --git a/runtime/include/tcp_session.h b/runtime/include/tcp_session.h index ddb32b9..8baab85 100644 --- a/runtime/include/tcp_session.h +++ b/runtime/include/tcp_session.h @@ -37,7 +37,7 @@ typedef void (*void_star_cb)(void *); * @param client_socket - the client * @param buffer - buffer to write to socket * @param on_eagain - cb to execute when client socket returns EAGAIN. If NULL, error out - * @returns nwritten on success, -1 on error, -2 unused, -3 on eagain + * @returns nwritten on success, -errno, -EAGAIN on block */ static inline ssize_t tcp_session_send(int client_socket, const char *buffer, size_t buffer_len, void_star_cb on_eagain, void *dataptr) @@ -49,9 +49,9 @@ tcp_session_send(int client_socket, const char *buffer, size_t buffer_len, void_ if (sent < 0) { if (errno == EAGAIN || errno == EWOULDBLOCK) { if (on_eagain != NULL) on_eagain(dataptr); - return -3; + return -EAGAIN; } else { - return -1; + return -errno; } } @@ -64,7 +64,7 @@ tcp_session_send(int client_socket, const char *buffer, size_t buffer_len, void_ * @param buffer - buffer to reach the socket into * @param buffer_len - buffer to reach the socket into * @param on_eagain - cb to execute when client socket returns EAGAIN. If NULL, error out - * @returns nwritten on success, -1 on error, -2 unused, -3 on eagain + * @returns nwritten on success, -errno on error, -eagain on block */ static inline ssize_t tcp_session_recv(int client_socket, char *buffer, size_t buffer_len, void_star_cb on_eagain, void *dataptr) @@ -76,9 +76,9 @@ tcp_session_recv(int client_socket, char *buffer, size_t buffer_len, void_star_c if (received < 0) { if (errno == EAGAIN || errno == EWOULDBLOCK) { if (on_eagain != NULL) on_eagain(dataptr); - return -3; + return -EAGAIN; } else { - return -1; + return -errno; } } diff --git a/runtime/include/wasm_stack.h b/runtime/include/wasm_stack.h index 1821172..95d5ac3 100644 --- a/runtime/include/wasm_stack.h +++ b/runtime/include/wasm_stack.h @@ -4,6 +4,7 @@ #include #include #include +#include #include "ps_list.h" #include "types.h" @@ -124,6 +125,6 @@ wasm_stack_reinit(struct wasm_stack *wasm_stack) assert(wasm_stack->low == wasm_stack->buffer + /* guard page */ PAGE_SIZE); assert(wasm_stack->high == wasm_stack->low + wasm_stack->capacity); - memset(wasm_stack->low, 0, wasm_stack->capacity); + explicit_bzero(wasm_stack->low, wasm_stack->capacity); ps_list_init_d(wasm_stack); } diff --git a/runtime/src/current_sandbox.c b/runtime/src/current_sandbox.c index ef6469f..1d3f43e 100644 --- a/runtime/src/current_sandbox.c +++ b/runtime/src/current_sandbox.c @@ -170,9 +170,7 @@ current_sandbox_fini() sandbox_syscall(sandbox); sandbox->timestamp_of.completion = __getcycles(); - sandbox->total_time = sandbox->timestamp_of.completion - sandbox->timestamp_of.request_arrival; - - sandbox->timestamp_of.response = __getcycles(); + sandbox->total_time = sandbox->timestamp_of.completion - sandbox->timestamp_of.allocation; assert(sandbox->state == SANDBOX_RUNNING_SYS); worker_thread_epoll_remove_sandbox(sandbox); diff --git a/runtime/src/listener_thread.c b/runtime/src/listener_thread.c index 5ab1c65..4553487 100644 --- a/runtime/src/listener_thread.c +++ b/runtime/src/listener_thread.c @@ -178,17 +178,17 @@ on_client_request_receiving(struct http_session *session) if (likely(rc == 0)) { on_client_request_received(session); return; - } else if (rc == -3) { + } else if (unlikely(rc == -EAGAIN)) { /* session blocked and registered to epoll so continue to next handle */ return; - } else if (rc == -2) { + } else if (unlikely(rc == -ENOMEM)) { /* Failed to grow request buffer */ debuglog("Failed to grow http request buffer\n"); session->state = HTTP_SESSION_EXECUTION_COMPLETE; http_session_set_response_header(session, 500, NULL, 0); on_client_response_header_sending(session); return; - } else if (rc == -1) { + } else if (rc < 0) { debuglog("Failed to receive or parse request\n"); session->state = HTTP_SESSION_EXECUTION_COMPLETE; http_session_set_response_header(session, 400, NULL, 0); @@ -257,11 +257,12 @@ on_client_response_header_sending(struct http_session *session) if (likely(rc == 0)) { on_client_response_body_sending(session); return; - } else if (rc == -3) { + } else if (unlikely(rc == -EAGAIN)) { /* session blocked and registered to epoll so continue to next handle */ return; - } else if (rc == -1) { + } else if (rc < 0) { http_session_close(session); + http_session_free(session); return; } } @@ -274,12 +275,12 @@ on_client_response_body_sending(struct http_session *session) if (likely(rc == 0)) { on_client_response_sent(session); return; - } - if (rc == -3) { + } else if (unlikely(rc == -EAGAIN)) { /* session blocked and registered to epoll so continue to next handle */ return; - } else if (rc == -1) { + } else if (unlikely(rc < 0)) { http_session_close(session); + http_session_free(session); return; } } @@ -287,6 +288,8 @@ on_client_response_body_sending(struct http_session *session) static void on_client_response_sent(struct http_session *session) { + session->response_sent_timestamp = __getcycles(); + http_session_close(session); http_session_free(session); return; diff --git a/runtime/src/sandbox.c b/runtime/src/sandbox.c index 88c44a0..88cf3f8 100644 --- a/runtime/src/sandbox.c +++ b/runtime/src/sandbox.c @@ -153,8 +153,7 @@ sandbox_init(struct sandbox *sandbox, struct module *module, struct http_session sandbox->tenant = tenant; sandbox->route = route; - sandbox->timestamp_of.request_arrival = session->request_arrival_timestamp; - sandbox->absolute_deadline = session->request_arrival_timestamp + sandbox->route->relative_deadline; + sandbox->absolute_deadline = sandbox->timestamp_of.allocation + sandbox->route->relative_deadline; /* * Admissions Control State