lib: graph: remove useless checks, make functions inline on fast path
authorPhilippe Proulx <eeppeliteloop@gmail.com>
Tue, 12 Jun 2018 21:47:12 +0000 (17:47 -0400)
committerPhilippe Proulx <eeppeliteloop@gmail.com>
Thu, 2 May 2019 04:05:45 +0000 (00:05 -0400)
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 <eeppeliteloop@gmail.com>
include/babeltrace/graph/component-sink-internal.h
include/babeltrace/graph/graph-internal.h
include/babeltrace/graph/graph.h
lib/graph/graph.c
lib/graph/sink.c
tests/lib/test_bt_notification_iterator.c

index 34c899c15ec5bbb94f606ed5ecc028e1a1614786..3840bd509ae36356a54442d5c639802adf658bb6 100644 (file)
@@ -28,6 +28,7 @@
  */
 
 #include <babeltrace/babeltrace-internal.h>
+#include <babeltrace/compiler-internal.h>
 #include <babeltrace/graph/component-sink.h>
 #include <babeltrace/graph/component-internal.h>
 #include <babeltrace/graph/component-class-internal.h>
@@ -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 */
index 9e6012ec641e18225640869a8429db2205ec15fd..3907568f3865df774614f6e9ec3c8b6d3b08cec3 100644 (file)
@@ -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)";
        }
index acff83931a8b1a069f1b2e110bc58715ea9cf14e..31d9c4212e284c1fc104cfe92bd9b7406c7c9f74 100644 (file)
@@ -30,6 +30,9 @@
 /* For bt_bool */
 #include <babeltrace/types.h>
 
+/* For enum bt_component_status */
+#include <babeltrace/graph/component-status.h>
+
 #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,
index 7db5b761a32c1fc28baf4e1a0c77a50ca1281d70..e948c89528f0cdd6af0fba47e5ad7a8ae7e5335f 100644 (file)
@@ -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;
 }
index bee8403cc5a65772698072c5dc4969b3407c1ef4..9ef89d3fda38769d8615cb1dc04891a8af183d8e 100644 (file)
@@ -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;
index 4ccd88766c2382ce01f728c8e5e5b022a082e6d9..f076af51095e93dd808e873ce877e783bc24428d 100644 (file)
@@ -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();
 }
This page took 0.030755 seconds and 4 git commands to generate.