From efea7ad0607e12b1ba0c64f9d13c1cc58f3d6f54 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Sat, 6 May 2023 00:24:05 +0200 Subject: [PATCH] hooks: add and use _fast callback function Add a _fast callback function that skips the version and method check. We can use this in places where performance is critical when we do the check out of the critical loops. Make all system methods _fast calls. We expect them to exist and have the right version. If we add new versions we can make them slow. --- spa/include/spa/node/node.h | 12 ++++++++++++ spa/include/spa/support/loop.h | 16 ++++++++++++++-- spa/include/spa/support/system.h | 3 +-- spa/include/spa/utils/hook.h | 19 +++++++++++++++++++ spa/plugins/audioconvert/audioadapter.c | 16 ++++++++-------- spa/plugins/support/loop.c | 2 ++ src/modules/module-client-node/remote-node.c | 2 +- src/pipewire/data-loop.c | 14 +++----------- src/pipewire/impl-node.c | 8 ++++---- src/pipewire/loop.c | 6 ++++++ src/pipewire/loop.h | 2 +- test/test-loop.c | 5 +++++ 12 files changed, 76 insertions(+), 29 deletions(-) diff --git a/spa/include/spa/node/node.h b/spa/include/spa/node/node.h index 50f5c6dfb..933ef178f 100644 --- a/spa/include/spa/node/node.h +++ b/spa/include/spa/node/node.h @@ -632,6 +632,16 @@ struct spa_node_methods { _res; \ }) +#define spa_node_method_fast(o,method,version,...) \ +({ \ + int _res; \ + struct spa_node *_n = o; \ + spa_interface_call_fast_res(&_n->iface, \ + struct spa_node_methods, _res, \ + method, version, ##__VA_ARGS__); \ + _res; \ +}) + #define spa_node_add_listener(n,...) spa_node_method(n, add_listener, 0, __VA_ARGS__) #define spa_node_set_callbacks(n,...) spa_node_method(n, set_callbacks, 0, __VA_ARGS__) #define spa_node_sync(n,...) spa_node_method(n, sync, 0, __VA_ARGS__) @@ -647,7 +657,9 @@ struct spa_node_methods { #define spa_node_port_set_io(n,...) spa_node_method(n, port_set_io, 0, __VA_ARGS__) #define spa_node_port_reuse_buffer(n,...) spa_node_method(n, port_reuse_buffer, 0, __VA_ARGS__) +#define spa_node_port_reuse_buffer_fast(n,...) spa_node_method_fast(n, port_reuse_buffer, 0, __VA_ARGS__) #define spa_node_process(n) spa_node_method(n, process, 0) +#define spa_node_process_fast(n) spa_node_method_fast(n, process, 0) /** * \} diff --git a/spa/include/spa/support/loop.h b/spa/include/spa/support/loop.h index 0935c6243..01b9abc5b 100644 --- a/spa/include/spa/support/loop.h +++ b/spa/include/spa/support/loop.h @@ -123,7 +123,7 @@ struct spa_loop_control_hooks { struct spa_hook_list *_l = l; \ struct spa_hook *_h; \ spa_list_for_each_reverse(_h, &_l->list, link) \ - spa_callbacks_call(&_h->cb, struct spa_loop_control_hooks, before, 0); \ + spa_callbacks_call_fast(&_h->cb, struct spa_loop_control_hooks, before, 0); \ }) #define spa_loop_control_hook_after(l) \ @@ -131,7 +131,7 @@ struct spa_loop_control_hooks { struct spa_hook_list *_l = l; \ struct spa_hook *_h; \ spa_list_for_each(_h, &_l->list, link) \ - spa_callbacks_call(&_h->cb, struct spa_loop_control_hooks, after, 0); \ + spa_callbacks_call_fast(&_h->cb, struct spa_loop_control_hooks, after, 0); \ }) /** @@ -212,6 +212,16 @@ struct spa_loop_control_methods { _res; \ }) +#define spa_loop_control_method_fast_r(o,method,version,...) \ +({ \ + int _res; \ + struct spa_loop_control *_o = o; \ + spa_interface_call_fast_res(&_o->iface, \ + struct spa_loop_control_methods, _res, \ + method, version, ##__VA_ARGS__); \ + _res; \ +}) + #define spa_loop_control_get_fd(l) spa_loop_control_method_r(l,get_fd,0) #define spa_loop_control_add_hook(l,...) spa_loop_control_method_v(l,add_hook,0,__VA_ARGS__) #define spa_loop_control_enter(l) spa_loop_control_method_v(l,enter,0) @@ -219,6 +229,8 @@ struct spa_loop_control_methods { #define spa_loop_control_iterate(l,...) spa_loop_control_method_r(l,iterate,0,__VA_ARGS__) #define spa_loop_control_check(l) spa_loop_control_method_r(l,check,1) +#define spa_loop_control_iterate_fast(l,...) spa_loop_control_method_fast_r(l,iterate,0,__VA_ARGS__) + typedef void (*spa_source_io_func_t) (void *data, int fd, uint32_t mask); typedef void (*spa_source_idle_func_t) (void *data); typedef void (*spa_source_event_func_t) (void *data, uint64_t count); diff --git a/spa/include/spa/support/system.h b/spa/include/spa/support/system.h index ad7e886fa..9ea41bce8 100644 --- a/spa/include/spa/support/system.h +++ b/spa/include/spa/support/system.h @@ -101,13 +101,12 @@ struct spa_system_methods { ({ \ volatile int _res = -ENOTSUP; \ struct spa_system *_o = o; \ - spa_interface_call_res(&_o->iface, \ + spa_interface_call_fast_res(&_o->iface, \ struct spa_system_methods, _res, \ method, version, ##__VA_ARGS__); \ _res; \ }) - #define spa_system_read(s,...) spa_system_method_r(s,read,0,__VA_ARGS__) #define spa_system_write(s,...) spa_system_method_r(s,write,0,__VA_ARGS__) #define spa_system_ioctl(s,...) spa_system_method_r(s,ioctl,0,__VA_ARGS__) diff --git a/spa/include/spa/utils/hook.h b/spa/include/spa/utils/hook.h index 38d41113b..aea18d28e 100644 --- a/spa/include/spa/utils/hook.h +++ b/spa/include/spa/utils/hook.h @@ -162,6 +162,14 @@ struct spa_interface { _res; \ }) +#define spa_callbacks_call_fast(callbacks,type,method,vers,...) \ +({ \ + const type *_f = (const type *) (callbacks)->funcs; \ + _f->method((callbacks)->data, ## __VA_ARGS__); \ + true; \ +}) + + /** * True if the \a callbacks are of version \a vers, false otherwise */ @@ -194,6 +202,11 @@ struct spa_interface { res = _f->method((callbacks)->data, ## __VA_ARGS__); \ res; \ }) +#define spa_callbacks_call_fast_res(callbacks,type,res,method,vers,...) \ +({ \ + const type *_f = (const type *) (callbacks)->funcs; \ + res = _f->method((callbacks)->data, ## __VA_ARGS__); \ +}) /** * True if the \a iface's callbacks are of version \a vers, false otherwise @@ -216,6 +229,9 @@ struct spa_interface { #define spa_interface_call(iface,method_type,method,vers,...) \ spa_callbacks_call(&(iface)->cb,method_type,method,vers,##__VA_ARGS__) +#define spa_interface_call_fast(iface,method_type,method,vers,...) \ + spa_callbacks_call_fast(&(iface)->cb,method_type,method,vers,##__VA_ARGS__) + /** * Invoke method named \a method in the callbacks on the given interface object. * The \a method_type defines the type of the method struct, not the interface @@ -226,6 +242,9 @@ struct spa_interface { #define spa_interface_call_res(iface,method_type,res,method,vers,...) \ spa_callbacks_call_res(&(iface)->cb,method_type,res,method,vers,##__VA_ARGS__) +#define spa_interface_call_fast_res(iface,method_type,res,method,vers,...) \ + spa_callbacks_call_fast_res(&(iface)->cb,method_type,res,method,vers,##__VA_ARGS__) + /** * \} */ diff --git a/spa/plugins/audioconvert/audioadapter.c b/spa/plugins/audioconvert/audioadapter.c index 1bacdac57..517caf19e 100644 --- a/spa/plugins/audioconvert/audioadapter.c +++ b/spa/plugins/audioconvert/audioadapter.c @@ -1230,12 +1230,12 @@ static int follower_ready(void *data, int status) if (this->direction == SPA_DIRECTION_OUTPUT) { int retry = 8; while (retry--) { - status = spa_node_process(this->convert); + status = spa_node_process_fast(this->convert); if (status & SPA_STATUS_HAVE_DATA) break; if (status & SPA_STATUS_NEED_DATA) { - status = spa_node_process(this->follower); + status = spa_node_process_fast(this->follower); if (!(status & SPA_STATUS_HAVE_DATA)) break; } @@ -1480,7 +1480,7 @@ static int impl_node_process(void *object) if (this->target == this->follower) { if (this->io_position) this->io_rate_match.size = this->io_position->clock.duration; - return spa_node_process(this->follower); + return spa_node_process_fast(this->follower); } if (this->direction == SPA_DIRECTION_INPUT) { @@ -1488,7 +1488,7 @@ static int impl_node_process(void *object) * First we run the converter to process the input for the follower * then if it produced data, we run the follower. */ while (retry--) { - status = spa_node_process(this->convert); + status = spa_node_process_fast(this->convert); /* schedule the follower when the converter needed * a recycled buffer */ if (status == -EPIPE || status == 0) @@ -1499,7 +1499,7 @@ static int impl_node_process(void *object) if (status & (SPA_STATUS_HAVE_DATA | SPA_STATUS_DRAINED)) { /* as long as the converter produced something or * is drained, process the follower. */ - fstatus = spa_node_process(this->follower); + fstatus = spa_node_process_fast(this->follower); if (fstatus < 0) { status = fstatus; break; @@ -1520,7 +1520,7 @@ static int impl_node_process(void *object) /* output node (source). First run the converter to make * sure we push out any queued data. Then when it needs * more data, schedule the follower. */ - status = spa_node_process(this->convert); + status = spa_node_process_fast(this->convert); if (status == 0) status = SPA_STATUS_NEED_DATA; else if (status < 0) @@ -1537,7 +1537,7 @@ static int impl_node_process(void *object) if (status & SPA_STATUS_NEED_DATA) { /* the converter needs more data, schedule the * follower */ - fstatus = spa_node_process(this->follower); + fstatus = spa_node_process_fast(this->follower); if (fstatus < 0) { status = fstatus; break; @@ -1556,7 +1556,7 @@ static int impl_node_process(void *object) spa_node_call_xrun(&this->callbacks, 0, 0, NULL); } else { - status = spa_node_process(this->follower); + status = spa_node_process_fast(this->follower); } spa_log_trace_fp(this->log, "%p: process status:%d", this, status); diff --git a/spa/plugins/support/loop.c b/spa/plugins/support/loop.c index bae9f27ef..d9d24e2ba 100644 --- a/spa/plugins/support/loop.c +++ b/spa/plugins/support/loop.c @@ -319,6 +319,8 @@ loop_add_hook(void *object, void *data) { struct impl *impl = object; + spa_return_if_fail(SPA_CALLBACK_CHECK(hooks, before, 0)); + spa_return_if_fail(SPA_CALLBACK_CHECK(hooks, after, 0)); spa_hook_list_append(&impl->hooks_list, hook, hooks, data); } diff --git a/src/modules/module-client-node/remote-node.c b/src/modules/module-client-node/remote-node.c index 87c94b6a9..82ff95fa1 100644 --- a/src/modules/module-client-node/remote-node.c +++ b/src/modules/module-client-node/remote-node.c @@ -1191,7 +1191,7 @@ static int node_ready(void *d, int status) if (status & SPA_STATUS_HAVE_DATA) { spa_list_for_each(p, &node->rt.output_mix, rt.node_link) - spa_node_process(p->mix); + spa_node_process_fast(p->mix); } a->state[0].status = status; diff --git a/src/pipewire/data-loop.c b/src/pipewire/data-loop.c index 1ea4563e0..ff68ac759 100644 --- a/src/pipewire/data-loop.c +++ b/src/pipewire/data-loop.c @@ -51,18 +51,10 @@ static void *do_loop(void *user_data) { struct pw_data_loop *this = user_data; int res; - int (*iterate) (void *object, int timeout); - struct spa_interface *iface = &this->loop->control->iface; - struct spa_callbacks *cb = &iface->cb; - const struct spa_loop_control_methods *m = - (const struct spa_loop_control_methods *)cb->funcs; + struct spa_callbacks *cb = &this->loop->control->iface.cb; + const struct spa_loop_control_methods *m = cb->funcs; void *data = cb->data; - - if (!SPA_CALLBACK_CHECK(m, iterate, 0)) { - pw_log_error("loop is missing iterate method"); - return NULL; - } - iterate = m->iterate; + int (*iterate) (void *object, int timeout) = m->iterate; pw_log_debug("%p: enter thread", this); pw_loop_enter(this->loop); diff --git a/src/pipewire/impl-node.c b/src/pipewire/impl-node.c index e89b90102..6fc1a7dc1 100644 --- a/src/pipewire/impl-node.c +++ b/src/pipewire/impl-node.c @@ -1166,13 +1166,13 @@ static inline int process_node(void *data) if (SPA_LIKELY(this->added)) { spa_list_for_each(p, &this->rt.input_mix, rt.node_link) - spa_node_process(p->mix); + spa_node_process_fast(p->mix); - status = spa_node_process(this->node); + status = spa_node_process_fast(this->node); if (status & SPA_STATUS_HAVE_DATA) { spa_list_for_each(p, &this->rt.output_mix, rt.node_link) - spa_node_process(p->mix); + spa_node_process_fast(p->mix); } } else { /* This can happen when we deactivated the node but some links are @@ -1822,7 +1822,7 @@ again: /* remote nodes have done the output mix already before * they triggered the ready event */ spa_list_for_each(p, &node->rt.output_mix, rt.node_link) - spa_node_process(p->mix); + spa_node_process_fast(p->mix); } return resume_node(node, status, nsec); } diff --git a/src/pipewire/loop.c b/src/pipewire/loop.c index 0d9248e55..914dd459c 100644 --- a/src/pipewire/loop.c +++ b/src/pipewire/loop.c @@ -109,6 +109,12 @@ struct pw_loop *pw_loop_new(const struct spa_dict *props) goto error_unload_loop; } this->control = iface; + if (!spa_interface_callback_check(&this->control->iface, + struct spa_loop_control_methods, iterate, 0)) { + res = -EINVAL; + pw_log_error("%p: loop does not support iterate", this); + goto error_unload_loop; + } if ((res = spa_handle_get_interface(impl->loop_handle, SPA_TYPE_INTERFACE_LoopUtils, diff --git a/src/pipewire/loop.h b/src/pipewire/loop.h index 6769ab646..25f5cfb22 100644 --- a/src/pipewire/loop.h +++ b/src/pipewire/loop.h @@ -45,8 +45,8 @@ pw_loop_destroy(struct pw_loop *loop); #define pw_loop_get_fd(l) spa_loop_control_get_fd((l)->control) #define pw_loop_add_hook(l,...) spa_loop_control_add_hook((l)->control,__VA_ARGS__) #define pw_loop_enter(l) spa_loop_control_enter((l)->control) -#define pw_loop_iterate(l,...) spa_loop_control_iterate((l)->control,__VA_ARGS__) #define pw_loop_leave(l) spa_loop_control_leave((l)->control) +#define pw_loop_iterate(l,...) spa_loop_control_iterate_fast((l)->control,__VA_ARGS__) #define pw_loop_add_io(l,...) spa_loop_utils_add_io((l)->utils,__VA_ARGS__) #define pw_loop_update_io(l,...) spa_loop_utils_update_io((l)->utils,__VA_ARGS__) diff --git a/test/test-loop.c b/test/test-loop.c index 523f39031..be9563ae8 100644 --- a/test/test-loop.c +++ b/test/test-loop.c @@ -234,6 +234,10 @@ struct dmsbd_data { struct spa_hook hook; }; +static void dmsbd_before(void *data) +{ +} + static void dmsbd_after(void *data) { struct dmsbd_data *d = data; @@ -244,6 +248,7 @@ static void dmsbd_after(void *data) static const struct spa_loop_control_hooks dmsbd_hooks = { SPA_VERSION_LOOP_CONTROL_HOOKS, + .before = dmsbd_before, .after = dmsbd_after, };