From: Philippe Proulx Date: Tue, 12 Jun 2018 21:47:12 +0000 (-0400) Subject: lib: graph: remove useless checks, make functions inline on fast path X-Git-Url: http://git.efficios.com/?p=babeltrace.git;a=commitdiff_plain;h=ad8474550bfc1968366fe45818c046fa5bc15581 lib: graph: remove useless checks, make functions inline on fast path This patch: * Removes a few useless checks on the fast path for everything related to the graph object. * Makes the "can consume" flag only written and read in developer mode. * Makes fast path functions `static inline` within `graph.c`. Signed-off-by: Philippe Proulx --- diff --git a/include/babeltrace/graph/component-sink-internal.h b/include/babeltrace/graph/component-sink-internal.h index 34c899c1..3840bd50 100644 --- a/include/babeltrace/graph/component-sink-internal.h +++ b/include/babeltrace/graph/component-sink-internal.h @@ -28,6 +28,7 @@ */ #include +#include #include #include #include @@ -54,14 +55,31 @@ struct bt_component *bt_component_sink_create( BT_HIDDEN void bt_component_sink_destroy(struct bt_component *component); -/** - * Process one event, consuming from sources as needed. - * - * @param component Component instance - * @returns One of #bt_component_status values - */ -BT_HIDDEN +static inline enum bt_component_status bt_component_sink_consume( - struct bt_component *component); + struct bt_component *component) +{ + enum bt_component_status ret = BT_COMPONENT_STATUS_OK; + struct bt_component_class_sink *sink_class = NULL; + + BT_ASSERT(component); + BT_ASSERT(bt_component_get_class_type(component) == + BT_COMPONENT_CLASS_TYPE_SINK); + sink_class = container_of(component->class, + struct bt_component_class_sink, parent); + BT_ASSERT(sink_class->methods.consume); + BT_LOGD("Calling user's consume method: " + "comp-addr=%p, comp-name=\"%s\"", + component, bt_component_get_name(component)); + ret = sink_class->methods.consume( + bt_private_component_from_component(component)); + BT_LOGD("User method returned: status=%s", + bt_component_status_string(ret)); + if (ret < 0) { + BT_LOGW_STR("Consume method failed."); + } + + return ret; +} #endif /* BABELTRACE_GRAPH_COMPONENT_SINK_INTERNAL_H */ diff --git a/include/babeltrace/graph/graph-internal.h b/include/babeltrace/graph/graph-internal.h index 9e6012ec..3907568f 100644 --- a/include/babeltrace/graph/graph-internal.h +++ b/include/babeltrace/graph/graph-internal.h @@ -110,14 +110,17 @@ struct bt_graph { }; static inline -void bt_graph_set_can_consume(struct bt_graph *graph, bt_bool can_consume) +void _bt_graph_set_can_consume(struct bt_graph *graph, bt_bool can_consume) { BT_ASSERT(graph); graph->can_consume = can_consume; } -BT_HIDDEN -enum bt_graph_status bt_graph_consume_no_check(struct bt_graph *graph); +#ifdef BT_DEV_MODE +# define bt_graph_set_can_consume _bt_graph_set_can_consume +#else +# define bt_graph_set_can_consume(_graph, _can_consume) +#endif BT_HIDDEN enum bt_graph_status bt_graph_consume_sink_no_check(struct bt_graph *graph, @@ -183,8 +186,6 @@ const char *bt_graph_status_string(enum bt_graph_status status) return "BT_GRAPH_STATUS_COMPONENT_REFUSES_PORT_CONNECTION"; case BT_GRAPH_STATUS_NOMEM: return "BT_GRAPH_STATUS_NOMEM"; - case BT_GRAPH_STATUS_CANNOT_CONSUME: - return "BT_GRAPH_STATUS_CANNOT_CONSUME"; default: return "(unknown)"; } diff --git a/include/babeltrace/graph/graph.h b/include/babeltrace/graph/graph.h index acff8393..31d9c421 100644 --- a/include/babeltrace/graph/graph.h +++ b/include/babeltrace/graph/graph.h @@ -30,6 +30,9 @@ /* For bt_bool */ #include +/* For enum bt_component_status */ +#include + #ifdef __cplusplus extern "C" { #endif @@ -43,20 +46,19 @@ struct bt_value; enum bt_graph_status { BT_GRAPH_STATUS_COMPONENT_REFUSES_PORT_CONNECTION = 111, /** Canceled. */ - BT_GRAPH_STATUS_CANCELED = 125, + BT_GRAPH_STATUS_CANCELED = BT_COMPONENT_STATUS_GRAPH_IS_CANCELED, /** No sink can consume at the moment. */ - BT_GRAPH_STATUS_AGAIN = 11, + BT_GRAPH_STATUS_AGAIN = BT_COMPONENT_STATUS_AGAIN, /** Downstream component does not support multiple inputs. */ - BT_GRAPH_STATUS_END = 1, - BT_GRAPH_STATUS_OK = 0, + BT_GRAPH_STATUS_END = BT_COMPONENT_STATUS_END, + BT_GRAPH_STATUS_OK = BT_COMPONENT_STATUS_OK, /** Invalid arguments. */ - BT_GRAPH_STATUS_INVALID = -22, + BT_GRAPH_STATUS_INVALID = BT_COMPONENT_STATUS_INVALID, /** No sink in graph. */ BT_GRAPH_STATUS_NO_SINK = -6, /** General error. */ - BT_GRAPH_STATUS_ERROR = -1, - BT_GRAPH_STATUS_CANNOT_CONSUME = -2, - BT_GRAPH_STATUS_NOMEM = -12, + BT_GRAPH_STATUS_ERROR = BT_COMPONENT_STATUS_ERROR, + BT_GRAPH_STATUS_NOMEM = BT_COMPONENT_STATUS_NOMEM, }; typedef void (*bt_graph_port_added_listener)(struct bt_port *port, diff --git a/lib/graph/graph.c b/lib/graph/graph.c index 7db5b761..e948c895 100644 --- a/lib/graph/graph.c +++ b/lib/graph/graph.c @@ -230,7 +230,7 @@ struct bt_graph *bt_graph_create(void) goto error; } - graph->can_consume = BT_TRUE; + bt_graph_set_can_consume(graph, BT_TRUE); ret = init_listeners_array(&graph->listeners.port_added); if (ret) { BT_LOGE_STR("Cannot create the \"port added\" listener array."); @@ -341,7 +341,7 @@ enum bt_graph_status bt_graph_connect_ports(struct bt_graph *graph, goto end; } - graph->can_consume = BT_FALSE; + bt_graph_set_can_consume(graph, BT_FALSE); /* Ensure appropriate types for upstream and downstream ports. */ if (bt_port_get_type(upstream_port) != BT_PORT_TYPE_OUTPUT) { @@ -486,15 +486,15 @@ end: bt_put(downstream_component); bt_put(connection); if (graph) { - graph->can_consume = init_can_consume; + (void) init_can_consume; + bt_graph_set_can_consume(graph, init_can_consume); } return status; } -static +static inline enum bt_graph_status consume_graph_sink(struct bt_component *sink) { - enum bt_graph_status status = BT_GRAPH_STATUS_OK; enum bt_component_status comp_status; BT_ASSERT(sink); @@ -502,25 +502,14 @@ enum bt_graph_status consume_graph_sink(struct bt_component *sink) BT_LOGV("Consumed from sink: addr=%p, name=\"%s\", status=%s", sink, bt_component_get_name(sink), bt_component_status_string(comp_status)); - - switch (comp_status) { - case BT_COMPONENT_STATUS_OK: - break; - case BT_COMPONENT_STATUS_END: - status = BT_GRAPH_STATUS_END; - break; - case BT_COMPONENT_STATUS_AGAIN: - status = BT_GRAPH_STATUS_AGAIN; - break; - case BT_COMPONENT_STATUS_INVALID: - status = BT_GRAPH_STATUS_INVALID; - break; - default: - status = BT_GRAPH_STATUS_ERROR; - break; - } - - return status; + BT_ASSERT_PRE(comp_status == BT_COMPONENT_STATUS_OK || + comp_status == BT_COMPONENT_STATUS_END || + comp_status == BT_COMPONENT_STATUS_AGAIN || + comp_status == BT_COMPONENT_STATUS_ERROR || + comp_status == BT_COMPONENT_STATUS_NOMEM, + "Invalid component status returned by consuming function: " + "status=%s", bt_component_status_string(comp_status)); + return (enum bt_graph_status) comp_status; } /* @@ -528,7 +517,7 @@ enum bt_graph_status consume_graph_sink(struct bt_component *sink) * this function. This function adds it back to the queue if there's * still something to consume afterwards. */ -static +static inline enum bt_graph_status consume_sink_node(struct bt_graph *graph, GList *node) { @@ -537,7 +526,7 @@ enum bt_graph_status consume_sink_node(struct bt_graph *graph, sink = node->data; status = consume_graph_sink(sink); - if (status != BT_GRAPH_STATUS_END) { + if (unlikely(status != BT_GRAPH_STATUS_END)) { g_queue_push_tail_link(graph->sinks_to_consume, node); goto end; } @@ -591,7 +580,7 @@ end: return status; } -BT_HIDDEN +static inline enum bt_graph_status bt_graph_consume_no_check(struct bt_graph *graph) { enum bt_graph_status status = BT_GRAPH_STATUS_OK; @@ -602,7 +591,7 @@ enum bt_graph_status bt_graph_consume_no_check(struct bt_graph *graph) BT_ASSERT_PRE(graph->has_sink, "Graph has no sink component: %!+g", graph); - if (g_queue_is_empty(graph->sinks_to_consume)) { + if (unlikely(g_queue_is_empty(graph->sinks_to_consume))) { BT_LOGV_STR("Graph's sink queue is empty: end of graph."); status = BT_GRAPH_STATUS_END; goto end; @@ -626,9 +615,9 @@ enum bt_graph_status bt_graph_consume(struct bt_graph *graph) BT_ASSERT_PRE(!graph->canceled, "Graph is canceled: %!+g", graph); BT_ASSERT_PRE(graph->can_consume, "Cannot consume graph in its current state: %!+g", graph); - graph->can_consume = BT_FALSE; + bt_graph_set_can_consume(graph, BT_FALSE); status = bt_graph_consume_no_check(graph); - graph->can_consume = BT_TRUE; + bt_graph_set_can_consume(graph, BT_TRUE); return status; } @@ -649,13 +638,9 @@ enum bt_graph_status bt_graph_run(struct bt_graph *graph) goto end; } - if (!graph->can_consume) { - BT_LOGW_STR("Cannot run graph in its current state."); - status = BT_GRAPH_STATUS_CANNOT_CONSUME; - goto end; - } - - graph->can_consume = BT_FALSE; + BT_ASSERT_PRE(graph->can_consume, + "Cannot consume graph in its current state: %!+g", graph); + bt_graph_set_can_consume(graph, BT_FALSE); BT_LOGV("Running graph: addr=%p", graph); do { @@ -665,7 +650,7 @@ enum bt_graph_status bt_graph_run(struct bt_graph *graph) * signal, this is not a warning nor an error, it was * intentional: log with a DEBUG level only. */ - if (graph->canceled) { + if (unlikely(graph->canceled)) { BT_LOGD("Stopping the graph: graph is canceled: " "graph-addr=%p", graph); status = BT_GRAPH_STATUS_CANCELED; @@ -673,7 +658,7 @@ enum bt_graph_status bt_graph_run(struct bt_graph *graph) } status = bt_graph_consume_no_check(graph); - if (status == BT_GRAPH_STATUS_AGAIN) { + if (unlikely(status == BT_GRAPH_STATUS_AGAIN)) { /* * If AGAIN is received and there are multiple * sinks, go ahead and consume from the next @@ -700,7 +685,7 @@ enum bt_graph_status bt_graph_run(struct bt_graph *graph) end: BT_LOGV("Graph ran: status=%s", bt_graph_status_string(status)); if (graph) { - graph->can_consume = BT_TRUE; + bt_graph_set_can_consume(graph, BT_TRUE); } return status; } diff --git a/lib/graph/sink.c b/lib/graph/sink.c index bee8403c..9ef89d3f 100644 --- a/lib/graph/sink.c +++ b/lib/graph/sink.c @@ -60,30 +60,6 @@ end: return sink ? &sink->parent : NULL; } -BT_HIDDEN -enum bt_component_status bt_component_sink_consume( - struct bt_component *component) -{ - enum bt_component_status ret = BT_COMPONENT_STATUS_OK; - struct bt_component_class_sink *sink_class = NULL; - - BT_ASSERT(component); - BT_ASSERT(bt_component_get_class_type(component) == BT_COMPONENT_CLASS_TYPE_SINK); - sink_class = container_of(component->class, struct bt_component_class_sink, parent); - BT_ASSERT(sink_class->methods.consume); - BT_LOGD("Calling user's consume method: " - "comp-addr=%p, comp-name=\"%s\"", - component, bt_component_get_name(component)); - ret = sink_class->methods.consume(bt_private_component_from_component(component)); - BT_LOGD("User method returned: status=%s", - bt_component_status_string(ret)); - if (ret < 0) { - BT_LOGW_STR("Consume method failed."); - } - - return ret; -} - int64_t bt_component_sink_get_input_port_count(struct bt_component *component) { int64_t ret; diff --git a/tests/lib/test_bt_notification_iterator.c b/tests/lib/test_bt_notification_iterator.c index 4ccd8876..f076af51 100644 --- a/tests/lib/test_bt_notification_iterator.c +++ b/tests/lib/test_bt_notification_iterator.c @@ -60,7 +60,7 @@ #include "tap/tap.h" -#define NR_TESTS 6 +#define NR_TESTS 5 enum test { TEST_NO_AUTO_NOTIFS, @@ -921,40 +921,6 @@ void test_output_port_notification_iterator(void) bt_put(notif_iter); } -static -void test_output_port_notification_iterator_cannot_consume(void) -{ - struct bt_component *src_comp; - struct bt_notification_iterator *notif_iter; - struct bt_port *upstream_port; - - clear_test_events(); - current_test = TEST_OUTPUT_PORT_NOTIFICATION_ITERATOR; - diag("test: cannot consume graph with existing output port notification iterator"); - BT_ASSERT(!graph); - graph = bt_graph_create(); - BT_ASSERT(graph); - create_source_sink(graph, &src_comp, NULL); - - /* Create notification iterator on source's output port */ - upstream_port = bt_component_source_get_output_port_by_name(src_comp, "out"); - notif_iter = bt_output_port_notification_iterator_create(upstream_port, - NULL); - BT_ASSERT(notif_iter); - bt_put(upstream_port); - - /* - * This should fail because the graph is now managed by the - * notification iterator. - */ - ok(bt_graph_run(graph) == BT_GRAPH_STATUS_CANNOT_CONSUME, - "bt_graph_run() returns BT_GRAPH_STATUS_CANNOT_CONSUME when there's an output port notification iterator"); - - bt_put(src_comp); - BT_PUT(graph); - bt_put(notif_iter); -} - #define DEBUG_ENV_VAR "TEST_BT_NOTIFICATION_ITERATOR_DEBUG" int main(int argc, char **argv) @@ -967,7 +933,6 @@ int main(int argc, char **argv) init_static_data(); test_no_auto_notifs(); test_output_port_notification_iterator(); - test_output_port_notification_iterator_cannot_consume(); fini_static_data(); return exit_status(); }