[PATCH 2/5] client: Cleanup to hold lock for less time

spitzak at gmail.com spitzak at gmail.com
Tue May 17 06:18:07 UTC 2016


From: Bill Spitzak <spitzak at gmail.com>

Splits the allocation of wl_proxy from the assignment of the id. This removes
some code such as the malloc from the lock, possibly improving multithreaded
performance.

Removes unnecessary lock in wl_proxy_create_wrapper.

This does not change the public api.

Signed-off-by: Bill Spitzak <spitzak at gmail.com>
---
 src/wayland-client.c | 157 +++++++++++++++++++++++++--------------------------
 1 file changed, 77 insertions(+), 80 deletions(-)

diff --git a/src/wayland-client.c b/src/wayland-client.c
index 03c087a..9934cd2 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -325,24 +325,13 @@ wl_display_create_queue(struct wl_display *display)
 }
 
 static struct wl_proxy *
-proxy_create(struct wl_proxy *factory, const struct wl_interface *interface,
-	     uint32_t version)
+proxy_new(const struct wl_interface *interface)
 {
-	struct wl_proxy *proxy;
-	struct wl_display *display = factory->display;
-
-	proxy = zalloc(sizeof *proxy);
-	if (proxy == NULL)
-		return NULL;
-
-	proxy->object.interface = interface;
-	proxy->display = display;
-	proxy->queue = factory->queue;
-	proxy->refcount = 1;
-	proxy->version = version;
-
-	proxy->object.id = wl_map_insert_new(&display->objects, 0, proxy);
-
+	struct wl_proxy *proxy = zalloc(sizeof *proxy);
+	if (proxy) {
+		proxy->object.interface = interface;
+		proxy->refcount = 1;
+	}
 	return proxy;
 }
 
@@ -369,10 +358,16 @@ WL_EXPORT struct wl_proxy *
 wl_proxy_create(struct wl_proxy *factory, const struct wl_interface *interface)
 {
 	struct wl_display *display = factory->display;
-	struct wl_proxy *proxy;
+	struct wl_proxy *proxy = proxy_new(interface);
+	if (proxy == NULL)
+		return NULL;
+
+	proxy->display = display;
+	proxy->queue = factory->queue;
+	proxy->version = factory->version;
 
 	pthread_mutex_lock(&display->mutex);
-	proxy = proxy_create(factory, interface, factory->version);
+	proxy->object.id = wl_map_insert_new(&display->objects, 0, proxy);
 	pthread_mutex_unlock(&display->mutex);
 
 	return proxy;
@@ -383,19 +378,15 @@ static struct wl_proxy *
 wl_proxy_create_for_id(struct wl_proxy *factory,
 		       uint32_t id, const struct wl_interface *interface)
 {
-	struct wl_proxy *proxy;
 	struct wl_display *display = factory->display;
-
-	proxy = zalloc(sizeof *proxy);
+	struct wl_proxy *proxy = proxy_new(interface);
 	if (proxy == NULL)
 		return NULL;
 
-	proxy->object.interface = interface;
-	proxy->object.id = id;
 	proxy->display = display;
 	proxy->queue = factory->queue;
-	proxy->refcount = 1;
 	proxy->version = factory->version;
+	proxy->object.id = id;
 
 	wl_map_insert_at(&display->objects, 0, id, proxy);
 
@@ -539,33 +530,63 @@ wl_proxy_add_dispatcher(struct wl_proxy *proxy,
 	return 0;
 }
 
-static struct wl_proxy *
-create_outgoing_proxy(struct wl_proxy *proxy, const struct wl_message *message,
-		      union wl_argument *args,
-		      const struct wl_interface *interface, uint32_t version)
+/* Return index of the new_id argument */
+static int
+new_id_index(struct wl_proxy *proxy, uint32_t opcode)
 {
-	int i, count;
-	const char *signature;
-	struct argument_details arg;
-	struct wl_proxy *new_proxy = NULL;
-
-	signature = message->signature;
-	count = arg_count_for_signature(signature);
-	for (i = 0; i < count; i++) {
+	int i;
+	const struct wl_message *message =
+		&proxy->object.interface->methods[opcode];
+	const char *signature = message->signature;
+	int count = arg_count_for_signature(signature);
+	for (i = 0; ; i++) {
+		struct argument_details arg;
+		if (i >= count)
+			wl_abort("Missing new_id argument");
 		signature = get_next_argument(signature, &arg);
+		if (arg.type == 'n')
+			break;
+	}
+	return i;
+}
 
-		switch (arg.type) {
-		case 'n':
-			new_proxy = proxy_create(proxy, interface, version);
-			if (new_proxy == NULL)
-				return NULL;
+/* Does wl_proxy_marshal_array, but also allocates an id for new_proxy
+ * if it is not null before sending the message (which requires the
+ * lock that this holds).
+ */
+static void
+proxy_marshal_array(struct wl_proxy *proxy,
+		    uint32_t opcode,
+		    union wl_argument *args,
+		    struct wl_proxy *new_proxy)
+{
+	struct wl_closure *closure;
+	const struct wl_message *message;
 
-			args[i].o = &new_proxy->object;
-			break;
-		}
+	pthread_mutex_lock(&proxy->display->mutex);
+
+	if (new_proxy) {
+		if (new_proxy->object.id)
+			wl_abort("Proxy %p already has an id\n", new_proxy);
+		new_proxy->object.id =
+			wl_map_insert_new(&proxy->display->objects, 0, new_proxy);
 	}
 
-	return new_proxy;
+	message = &proxy->object.interface->methods[opcode];
+
+	closure = wl_closure_marshal(&proxy->object, opcode, args, message);
+	if (closure == NULL)
+		wl_abort("Error marshalling request: %s\n", strerror(errno));
+
+	if (debug_client)
+		wl_closure_print(closure, &proxy->object, true);
+
+	if (wl_closure_send(closure, proxy->display->connection))
+		wl_abort("Error sending request: %s\n", strerror(errno));
+
+	wl_closure_destroy(closure);
+
+	pthread_mutex_unlock(&proxy->display->mutex);
 }
 
 /** Prepare a request to be sent to the compositor
@@ -633,35 +654,19 @@ wl_proxy_marshal_array_constructor_versioned(struct wl_proxy *proxy,
 					     const struct wl_interface *interface,
 					     uint32_t version)
 {
-	struct wl_closure *closure;
 	struct wl_proxy *new_proxy = NULL;
-	const struct wl_message *message;
-
-	pthread_mutex_lock(&proxy->display->mutex);
 
-	message = &proxy->object.interface->methods[opcode];
 	if (interface) {
-		new_proxy = create_outgoing_proxy(proxy, message,
-						  args, interface,
-						  version);
+		new_proxy = proxy_new(interface);
 		if (new_proxy == NULL)
-			goto err_unlock;
+			return NULL;
+		new_proxy->display = proxy->display;
+		new_proxy->queue = proxy->queue;
+		new_proxy->version = version;
+		args[new_id_index(proxy, opcode)].o = &new_proxy->object;
 	}
 
-	closure = wl_closure_marshal(&proxy->object, opcode, args, message);
-	if (closure == NULL)
-		wl_abort("Error marshalling request: %s\n", strerror(errno));
-
-	if (debug_client)
-		wl_closure_print(closure, &proxy->object, true);
-
-	if (wl_closure_send(closure, proxy->display->connection))
-		wl_abort("Error sending request: %s\n", strerror(errno));
-
-	wl_closure_destroy(closure);
-
- err_unlock:
-	pthread_mutex_unlock(&proxy->display->mutex);
+	proxy_marshal_array(proxy, opcode, args, new_proxy);
 
 	return new_proxy;
 }
@@ -693,7 +698,7 @@ wl_proxy_marshal(struct wl_proxy *proxy, uint32_t opcode, ...)
 				 args, WL_CLOSURE_MAX_ARGS, ap);
 	va_end(ap);
 
-	wl_proxy_marshal_array_constructor(proxy, opcode, args, NULL);
+	proxy_marshal_array(proxy, opcode, args, NULL);
 }
 
 /** Prepare a request to be sent to the compositor
@@ -794,7 +799,7 @@ WL_EXPORT void
 wl_proxy_marshal_array(struct wl_proxy *proxy, uint32_t opcode,
 		       union wl_argument *args)
 {
-	wl_proxy_marshal_array_constructor(proxy, opcode, args, NULL);
+	proxy_marshal_array(proxy, opcode, args, NULL);
 }
 
 static void
@@ -2029,23 +2034,15 @@ WL_EXPORT void *
 wl_proxy_create_wrapper(void *proxy)
 {
 	struct wl_proxy *wrapped_proxy = proxy;
-	struct wl_proxy *wrapper;
-
-	wrapper = zalloc(sizeof *wrapper);
+	struct wl_proxy *wrapper = proxy_new(wrapped_proxy->object.interface);
 	if (!wrapper)
 		return NULL;
 
-	pthread_mutex_lock(&wrapped_proxy->display->mutex);
-
-	wrapper->object.interface = wrapped_proxy->object.interface;
 	wrapper->object.id = wrapped_proxy->object.id;
 	wrapper->version = wrapped_proxy->version;
 	wrapper->display = wrapped_proxy->display;
 	wrapper->queue = wrapped_proxy->queue;
 	wrapper->flags = WL_PROXY_FLAG_WRAPPER;
-	wrapper->refcount = 1;
-
-	pthread_mutex_unlock(&wrapped_proxy->display->mutex);
 
 	return wrapper;
 }
-- 
1.9.1



More information about the wayland-devel mailing list