From c3ef36d6b5d36f783f41b8860cee321b652f91e3 Mon Sep 17 00:00:00 2001 From: Ryan Dwyer Date: Wed, 15 Aug 2018 10:03:55 +1000 Subject: [PATCH] Simplify transactions Commit 4b8e3a885be74c588291c51f798de80bd81a92db makes it so only one transaction is committed (ie. configures sent) at a time. This commit removes the now-unnecessary code which was used to support concurrent committed transactions. * Instead of containers storing a list of instructions which they've been sent, it now stores a single instruction. * Containers now have an ntxnrefs property. Previously we knew how many references there were by the length of the instruction list. * Instructions no longer need a ready property. It was used to avoid marking an instruction ready twice when they were in a list, but this is now avoided because there is only one instruction and we nullify the container->instruction pointer when it's ready. * When a transaction applies, we no longer need to consider releasing and resaving the surface, as we know there are no other committed transactions. * transaction_notify_view_ready has been renamed to view_notify_view_ready_by_serial to make it consistent with transaction_notify_view_ready_by_size. * Out-of-memory checks have been added when creating transactions and instructions. --- include/sway/desktop/transaction.h | 5 +- include/sway/tree/container.h | 7 ++- sway/desktop/transaction.c | 99 ++++++++++-------------------- sway/desktop/xdg_shell.c | 5 +- sway/desktop/xdg_shell_v6.c | 5 +- sway/desktop/xwayland.c | 2 +- sway/tree/container.c | 6 +- sway/tree/root.c | 2 - 8 files changed, 51 insertions(+), 80 deletions(-) diff --git a/include/sway/desktop/transaction.h b/include/sway/desktop/transaction.h index 56361d94..7ac924e7 100644 --- a/include/sway/desktop/transaction.h +++ b/include/sway/desktop/transaction.h @@ -20,6 +20,8 @@ * create and commits a transaction from the dirty containers. */ +struct sway_transaction_instruction; + /** * Find all dirty containers, create and commit a transaction containing them, * and unmark them as dirty. @@ -31,7 +33,8 @@ void transaction_commit_dirty(void); * * When all views in the transaction are ready, the layout will be applied. */ -void transaction_notify_view_ready(struct sway_view *view, uint32_t serial); +void transaction_notify_view_ready_by_serial(struct sway_view *view, + uint32_t serial); /** * Notify the transaction system that a view is ready for the new layout, but diff --git a/include/sway/tree/container.h b/include/sway/tree/container.h index 2a22f196..b64a2e63 100644 --- a/include/sway/tree/container.h +++ b/include/sway/tree/container.h @@ -151,7 +151,12 @@ struct sway_container { struct wlr_texture *title_urgent; size_t title_height; - list_t *instructions; // struct sway_transaction_instruction * + // The number of transactions which reference this container. + size_t ntxnrefs; + + // If this container is a view and is waiting for the client to respond to a + // configure then this will be populated, otherwise NULL. + struct sway_transaction_instruction *instruction; bool destroying; diff --git a/sway/desktop/transaction.c b/sway/desktop/transaction.c index a449ffaa..c08730ce 100644 --- a/sway/desktop/transaction.c +++ b/sway/desktop/transaction.c @@ -43,12 +43,14 @@ struct sway_transaction_instruction { struct sway_container *container; struct sway_container_state state; uint32_t serial; - bool ready; }; static struct sway_transaction *transaction_create() { struct sway_transaction *transaction = calloc(1, sizeof(struct sway_transaction)); + if (!sway_assert(transaction, "Unable to allocate transaction")) { + return NULL; + } transaction->instructions = create_list(); if (server.debug_txn_timings) { clock_gettime(CLOCK_MONOTONIC, &transaction->create_time); @@ -62,13 +64,11 @@ static void transaction_destroy(struct sway_transaction *transaction) { struct sway_transaction_instruction *instruction = transaction->instructions->items[i]; struct sway_container *con = instruction->container; - for (int j = 0; j < con->instructions->length; ++j) { - if (con->instructions->items[j] == instruction) { - list_del(con->instructions, j); - break; - } + con->ntxnrefs--; + if (con->instruction == instruction) { + con->instruction = NULL; } - if (con->destroying && !con->instructions->length) { + if (con->destroying && con->ntxnrefs == 0) { container_free(con); } free(instruction); @@ -130,12 +130,16 @@ static void transaction_add_container(struct sway_transaction *transaction, struct sway_container *container) { struct sway_transaction_instruction *instruction = calloc(1, sizeof(struct sway_transaction_instruction)); + if (!sway_assert(instruction, "Unable to allocate instruction")) { + return; + } instruction->transaction = transaction; instruction->container = container; copy_pending_state(container, &instruction->state); list_add(transaction->instructions, instruction); + container->ntxnrefs++; } /** @@ -195,21 +199,11 @@ static void transaction_apply(struct sway_transaction *transaction) { memcpy(&container->current, &instruction->state, sizeof(struct sway_container_state)); - if (container->type == C_VIEW) { - if (container->destroying) { - if (container->instructions->length == 1 && - container->sway_view->saved_buffer) { - view_remove_saved_buffer(container->sway_view); - } - } else { - if (container->sway_view->saved_buffer) { - view_remove_saved_buffer(container->sway_view); - } - if (container->instructions->length > 1) { - view_save_buffer(container->sway_view); - } - } + if (container->type == C_VIEW && container->sway_view->saved_buffer) { + view_remove_saved_buffer(container->sway_view); } + + container->instruction = NULL; } } @@ -269,20 +263,8 @@ static bool should_configure(struct sway_container *con, if (con->destroying) { return false; } - // The settled dimensions are what size the view will be once any pending - // configures have applied (excluding the one we might be configuring now). - // If these match the dimensions that this transaction wants then we don't - // need to configure it. - int settled_width = con->current.view_width; - int settled_height = con->current.view_height; - if (con->instructions->length) { - struct sway_transaction_instruction *last_instruction = - con->instructions->items[con->instructions->length - 1]; - settled_width = last_instruction->state.view_width; - settled_height = last_instruction->state.view_height; - } - if (settled_width == instruction->state.view_width && - settled_height == instruction->state.view_height) { + if (con->current.view_width == instruction->state.view_width && + con->current.view_height == instruction->state.view_height) { return false; } return true; @@ -312,10 +294,10 @@ static void transaction_commit(struct sway_transaction *transaction) { struct timespec when; wlr_surface_send_frame_done(con->sway_view->surface, &when); } - if (con->type == C_VIEW && !con->sway_view->saved_buffer) { + if (con->type == C_VIEW) { view_save_buffer(con->sway_view); } - list_add(con->instructions, instruction); + con->instruction = instruction; } transaction->num_configures = transaction->num_waiting; if (server.debug_txn_timings) { @@ -347,7 +329,6 @@ static void transaction_commit(struct sway_transaction *transaction) { static void set_instruction_ready( struct sway_transaction_instruction *instruction) { - instruction->ready = true; struct sway_transaction *transaction = instruction->transaction; if (server.debug_txn_timings) { @@ -371,44 +352,25 @@ static void set_instruction_ready( wl_event_source_timer_update(transaction->timer, 0); } } -} -/** - * Mark all of the view's instructions as ready up to and including the - * instruction at the given index. This allows the view to skip a configure. - */ -static void set_instructions_ready(struct sway_view *view, int index) { - for (int i = 0; i <= index; ++i) { - struct sway_transaction_instruction *instruction = - view->swayc->instructions->items[i]; - if (!instruction->ready) { - set_instruction_ready(instruction); - } - } + instruction->container->instruction = NULL; transaction_progress_queue(); } -void transaction_notify_view_ready(struct sway_view *view, uint32_t serial) { - for (int i = 0; i < view->swayc->instructions->length; ++i) { - struct sway_transaction_instruction *instruction = - view->swayc->instructions->items[i]; - if (instruction->serial == serial && !instruction->ready) { - set_instructions_ready(view, i); - return; - } +void transaction_notify_view_ready_by_serial(struct sway_view *view, + uint32_t serial) { + struct sway_transaction_instruction *instruction = view->swayc->instruction; + if (view->swayc->instruction->serial == serial) { + set_instruction_ready(instruction); } } void transaction_notify_view_ready_by_size(struct sway_view *view, int width, int height) { - for (int i = 0; i < view->swayc->instructions->length; ++i) { - struct sway_transaction_instruction *instruction = - view->swayc->instructions->items[i]; - if (!instruction->ready && instruction->state.view_width == width && - instruction->state.view_height == height) { - set_instructions_ready(view, i); - return; - } + struct sway_transaction_instruction *instruction = view->swayc->instruction; + if (instruction->state.view_width == width && + instruction->state.view_height == height) { + set_instruction_ready(instruction); } } @@ -417,6 +379,9 @@ void transaction_commit_dirty(void) { return; } struct sway_transaction *transaction = transaction_create(); + if (!transaction) { + return; + } for (int i = 0; i < server.dirty_containers->length; ++i) { struct sway_container *container = server.dirty_containers->items[i]; transaction_add_container(transaction, container); diff --git a/sway/desktop/xdg_shell.c b/sway/desktop/xdg_shell.c index af9d49b8..6a7a3f7f 100644 --- a/sway/desktop/xdg_shell.c +++ b/sway/desktop/xdg_shell.c @@ -254,8 +254,9 @@ static void handle_commit(struct wl_listener *listener, void *data) { return; } - if (view->swayc->instructions->length) { - transaction_notify_view_ready(view, xdg_surface->configure_serial); + if (view->swayc->instruction) { + transaction_notify_view_ready_by_serial(view, + xdg_surface->configure_serial); } view_damage_from(view); diff --git a/sway/desktop/xdg_shell_v6.c b/sway/desktop/xdg_shell_v6.c index c6ac0f4e..5b3c7b2b 100644 --- a/sway/desktop/xdg_shell_v6.c +++ b/sway/desktop/xdg_shell_v6.c @@ -250,8 +250,9 @@ static void handle_commit(struct wl_listener *listener, void *data) { if (!view->swayc) { return; } - if (view->swayc->instructions->length) { - transaction_notify_view_ready(view, xdg_surface_v6->configure_serial); + if (view->swayc->instruction) { + transaction_notify_view_ready_by_serial(view, + xdg_surface_v6->configure_serial); } view_damage_from(view); diff --git a/sway/desktop/xwayland.c b/sway/desktop/xwayland.c index 5e8afa65..14f59b9c 100644 --- a/sway/desktop/xwayland.c +++ b/sway/desktop/xwayland.c @@ -284,7 +284,7 @@ static void handle_commit(struct wl_listener *listener, void *data) { struct wlr_xwayland_surface *xsurface = view->wlr_xwayland_surface; struct wlr_surface_state *surface_state = &xsurface->surface->current; - if (view->swayc->instructions->length) { + if (view->swayc->instruction) { transaction_notify_view_ready_by_size(view, surface_state->width, surface_state->height); } else if (container_is_floating(view->swayc)) { diff --git a/sway/tree/container.c b/sway/tree/container.c index 45e54080..eb06edc2 100644 --- a/sway/tree/container.c +++ b/sway/tree/container.c @@ -108,7 +108,6 @@ struct sway_container *container_create(enum sway_container_type type) { c->layout = L_NONE; c->type = type; c->alpha = 1.0f; - c->instructions = create_list(); if (type != C_VIEW) { c->children = create_list(); @@ -140,8 +139,8 @@ void container_free(struct sway_container *cont) { "Tried to free container which wasn't marked as destroying")) { return; } - if (!sway_assert(cont->instructions->length == 0, - "Tried to free container with pending instructions")) { + if (!sway_assert(cont->ntxnrefs == 0, "Tried to free container " + "which is still referenced by transactions")) { return; } free(cont->name); @@ -150,7 +149,6 @@ void container_free(struct sway_container *cont) { wlr_texture_destroy(cont->title_focused_inactive); wlr_texture_destroy(cont->title_unfocused); wlr_texture_destroy(cont->title_urgent); - list_free(cont->instructions); list_free(cont->children); list_free(cont->current.children); diff --git a/sway/tree/root.c b/sway/tree/root.c index 79f2194e..068e7911 100644 --- a/sway/tree/root.c +++ b/sway/tree/root.c @@ -26,7 +26,6 @@ void root_create(void) { root_container.type = C_ROOT; root_container.layout = L_NONE; root_container.name = strdup("root"); - root_container.instructions = create_list(); root_container.children = create_list(); root_container.current.children = create_list(); wl_signal_init(&root_container.events.destroy); @@ -55,7 +54,6 @@ void root_destroy(void) { free(root_container.sway_root); // root_container - list_free(root_container.instructions); list_free(root_container.children); list_free(root_container.current.children); free(root_container.name);