[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 09 of 10] xenalyze: add a basic plugin infrastructure



On 31/05/12 12:16, David Vrabel wrote:
Allow xenalyze to be include (at build time) plugins that can do
per-record actions and a summary.  These plugins can be in C or C++.

The plugins entry points are in struct plugin and pointers to all the
plugins linked in xenalyze are placed in a "plugin" section so
plugin_init() can find them all.

A new command line option (-p, --plugin=PLUGIN) is added to enable one
or more plugins.

A sample plugin (skeleton) is included (mostly because at least one
plugin must be present for the build to work).

Signed-off-by: David Vrabel<david.vrabel@xxxxxxxxxx>
So what's the main motivation of having this plugin infrastructure? The one plugin example you have ("batch-size", patch 10) seems simple enough that it should be fairly straightforward to just add as an option, with not much more boilerplate than C++ already requires.

Looks like potential advantages may include:
* Ability to use C++ language (for those who care for such things)
* Ability to use STL for complex data structures
* Ability to add an option like the "batch-size" plugin in a concise, self-contained way

Potential disadvantages include:
* An extra O(N) loop on the hot path (where N = # of enabled plugins)
* For each enabled plugin, an extra full function call on the hot path; and a C++ function at that, which (my prejudice tells me) is likely to be more wasteful time and space-wise than a C function.

Did I miss anything?

Obviosuly thanks for being conscious of this cost by, for example, having a separate list for "enabled" vs "available".

I guess I want to be convinced that allowing this kind of plug-in is really worth it, and won't cost too much; especially when something like "batch-size" could be implemented in a pretty straightforward way without needing a plugin. I suppose that if there's a plugin that turns out to be useful, but is expensive when run as a plug-in, we could always pull it into the main file as an optimization.

Also, could you do a simple test to measure the overhead of having no plugins? Just finding a 700MB-ish file you have lying around and running "xenalyze -s" with and with this patch (and with this patch and -p skeleton or -p batch-size) would be pretty informative, and shouldn't take too long.

Thanks,
 -George

---
  Makefile            |   10 +++---
  analyze.h           |   55 ++++++++++++++++++++++++++++++++++
  plugin.cc           |   84 
++++++++++++++++++++++++++++++++++++++++++++++++++++
  plugin.h            |   48 +++++++++++++++++++++++++++++
  plugin.hh           |   42 ++++++++++++++++++++++++++
  plugins/skeleton.cc |   31 +++++++++++++++++++
  xenalyze.c          |   72 +++++++++++++-------------------------------
  7 files changed, 287 insertions(+), 55 deletions(-)

diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -1,4 +1,4 @@
-CFLAGS += -g -O2
+CFLAGS += -g -O2 -I.
  CFLAGS += -fno-strict-aliasing
  CFLAGS += -std=gnu99
  CFLAGS += -Wall -Wstrict-prototypes
@@ -9,11 +9,15 @@ CFLAGS += -D_LARGEFILE_SOURCE -D_LARGEFI
  CFLAGS += -mno-tls-direct-seg-refs
  CFLAGS += -Werror

+CXXFLAGS := -g -O2 -I. -Wall -Werror -std=c++0x
+
  BIN := xenalyze dump-raw

  xenalyze_OBJS := \
        mread.o \
-       xenalyze.o
+       plugin.o \
+       xenalyze.o \
+       plugins/skeleton.o

  dump-raw_OBJS := \
        dump-raw.o
@@ -21,7 +25,7 @@ dump-raw_OBJS := \
  all: $(BIN)

  xenalyze: $(xenalyze_OBJS)
-       $(CC) $(LDFLAGS) -o $@ $^
+       $(CXX) $(LDFLAGS) -o $@ $^

  dump-raw: $(dump-raw_OBJS)
        $(CC) $(LDFLAGS) -o $@ $^
@@ -29,6 +33,9 @@ dump-raw: $(dump-raw_OBJS)
  %.o: %.c
        $(CC) $(CFLAGS) -MD -MP -c -o $@ $<

+%.o: %.cc
+       $(CXX) $(CXXFLAGS) -MD -MP -c -o $@ $<
+
  all_objs := $(foreach b,$(BIN),$($(b)_OBJS))
  all_deps := $(all_objs:.o=.d)

diff --git a/plugin.cc b/plugin.cc
new file mode 100644
--- /dev/null
+++ b/plugin.cc
@@ -0,0 +1,84 @@
+/*
+ * Xenalyze plugin infrastructure.
+ *
+ * Copyright (C) 2012, Citrix Systems R&D Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+#include<stdio.h>
+#include<string.h>
+#include<list>
+
+#include "plugin.hh"
+
+typedef std::list<struct plugin *>  plugin_list;
+
+static plugin_list available;
+static plugin_list enabled;
+
+bool plugin_enable(const char *name)
+{
+    for (auto p = available.begin(); p != available.end(); p++) {
+        struct plugin *plugin = *p;
+        if (strcmp(plugin->name, name) == 0) {
+            enabled.push_back(plugin);
+            if (plugin->enable) {
+                plugin->enable(plugin);
+            }
+            return true;
+        }
+    }
+    return false;
+}
+
+void plugin_process(const struct record_info *ri)
+{
+    for (auto p = enabled.begin(); p != enabled.end(); p++) {
+        struct plugin *plugin = *p;
+        if (plugin->process) {
+            plugin->process(plugin, ri);
+        }
+    }
+}
+
+void plugin_summary(void)
+{
+    for (auto p = enabled.begin(); p != enabled.end(); p++) {
+        struct plugin *plugin = *p;
+        if (plugin->summary) {
+            printf("Summary for %s plugin:\n", plugin->name);
+            plugin->summary(plugin);
+        }
+    }
+}
+
+static void plugin_add(struct plugin *plugin)
+{
+    available.push_back(plugin);
+}
+
+void plugin_init(void)
+{
+    extern struct plugin *__start_plugin;
+    extern struct plugin *__stop_plugin;
+    struct plugin **p;
+
+    for (p =&__start_plugin; p<  &__stop_plugin; p++) {
+        plugin_add(*p);
+    }
+}
+
+void plugin_process_wrapper(struct plugin *plugin, const struct record_info 
*ri)
+{
+    xenalyze_plugin *p = static_cast<xenalyze_plugin*>(plugin->data);
+    p->process(ri);
+}
+
+void plugin_summary_wrapper(struct plugin *plugin)
+{
+    xenalyze_plugin *p = static_cast<xenalyze_plugin*>(plugin->data);
+    p->summary();
+    delete p;
+}
diff --git a/plugin.h b/plugin.h
new file mode 100644
--- /dev/null
+++ b/plugin.h
@@ -0,0 +1,47 @@
+/*
+ * Xenalyze plugin C API.
+ *
+ * Copyright (C) 2012, Citrix Systems R&D Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+#ifndef PLUGIN_H
+#define PLUGIN_H
+
+#include<stdbool.h>
+
+#include "analyze.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct plugin;
+
+typedef void (*plugin_enable_f)(struct plugin *plugin);
+typedef void (*plugin_process_f)(struct plugin *plugin, const struct 
record_info *ri);
+typedef void (*plugin_summary_f)(struct plugin *plugin);
+
+struct plugin {
+    const char *name;
+    plugin_enable_f enable;
+    plugin_process_f process;
+    plugin_summary_f summary;
+    void *data;
+};
+
+#define DEFINE_PLUGIN(p) \
+    struct plugin *__plugin_ ## p __attribute__((section("plugin"))) =&p
+
+void plugin_init(void);
+bool plugin_enable(const char *name);
+void plugin_process(const struct record_info *ri);
+void plugin_summary(void);
+
+#ifdef __cplusplus
+} /* extern "C" */
+#endif
+
+#endif /* #ifndef PLUGIN_H */
diff --git a/plugin.hh b/plugin.hh
new file mode 100644
--- /dev/null
+++ b/plugin.hh
@@ -0,0 +1,42 @@
+/*
+ * Xenalyze plugin C++ API.
+ *
+ * Copyright (C) 2012, Citrix Systems R&D Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+#ifndef PLUGIN_HH
+#define PLUGIN_HH
+
+#include "plugin.h"
+
+class xenalyze_plugin {
+public:
+    virtual ~xenalyze_plugin() {}
+
+    virtual void process(const struct record_info *ri) = 0;
+    virtual void summary() = 0;
+};
+
+#define DEFINE_CXX_PLUGIN(name, cls)                                    \
+    static void __plugin_ ## cls ## _enable(struct plugin *plugin)      \
+    {                                                                   \
+        plugin->data = new cls();                                       \
+    }                                                                   \
+                                                                        \
+    static struct plugin __plugin ## cls = {                            \
+        name,                                                           \
+        __plugin_ ## cls ## _enable,                                    \
+        plugin_process_wrapper,                                         \
+        plugin_summary_wrapper,                                         \
+    };                                                                  \
+    DEFINE_PLUGIN(__plugin ## cls)
+
+extern "C" {
+void plugin_process_wrapper(struct plugin *plugin, const struct record_info 
*ri);
+void plugin_summary_wrapper(struct plugin *plugin);
+}
+
+#endif /* #ifndef PLUGIN_HH */
diff --git a/plugins/skeleton.cc b/plugins/skeleton.cc
new file mode 100644
--- /dev/null
+++ b/plugins/skeleton.cc
@@ -0,0 +1,31 @@
+/*
+ * Skeleton xenalyze plugin.
+ *
+ * Copyright (C) 2012, Citrix Systems R&D Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ */
+#include "plugin.hh"
+
+class skeleton_plugin : xenalyze_plugin {
+public:
+    skeleton_plugin() {}
+    ~skeleton_plugin() {}
+
+    void process(const struct record_info *ri);
+    void summary(void);
+};
+
+void skeleton_plugin::process(const struct record_info *ri)
+{
+    /* Put per-trace record stuff here. */
+}
+
+void skeleton_plugin::summary(void)
+{
+    /* Print a summary of the results (if applicable). */
+}
+
+DEFINE_CXX_PLUGIN("skeleton", skeleton_plugin);
diff --git a/xenalyze.c b/xenalyze.c
--- a/xenalyze.c
+++ b/xenalyze.c
@@ -32,6 +32,7 @@
  #include "trace.h"
  #include "analyze.h"
  #include "mread.h"
+#include "plugin.h"
  #include "pv.h"
  #include<errno.h>
  #include<strings.h>
@@ -8763,6 +8764,8 @@ void process_record(struct pcpu_info *p)
          default:
              process_generic(ri);
          }
+
+        plugin_process(ri);
      }

      UPDATE_VOLUME(p, toplevel[toplevel], ri->size);
@@ -9346,6 +9349,7 @@ enum {
      OPT_DUMP_ALL='a',
      OPT_INTERVAL_LENGTH='i',
      OPT_SUMMARY='s',
+    OPT_PLUGIN='p',
  };

  enum {
@@ -9816,6 +9820,15 @@ error_t cmd_parser(int key, char *arg, s
          opt.tsc_loop_fatal = 1;
          break;

+    case OPT_PLUGIN:
+        if (plugin_enable(arg)) {
+            G.output_defined = 1;
+        } else {
+            fprintf(stderr, "ERROR: No such plugin `%s'.\n", arg);
+            exit(1);
+        }
+        break;
+
      case ARGP_KEY_ARG:
      {
          /* FIXME - strcpy */
@@ -10108,6 +10121,10 @@ const struct argp_option cmd_opts[] =  {
        .arg = "errlevel",
        .doc = "Sets tolerance for errors found in the file.  Default is 3; max is 
6.", },

+    { .name = "plugin",
+      .key = OPT_PLUGIN,
+      .arg = "PLUGIN",
+      .doc = "Enable a decoder or summary plugin.", },

      { 0 },
  };
@@ -10127,6 +10144,8 @@ int main(int argc, char *argv[]) {
      /* Start with warn at stderr. */
      warn = stderr;

+    plugin_init();
+
      argp_parse(&parser_def, argc, argv, 0, NULL, NULL);

      if (G.trace_file == NULL)
@@ -10163,6 +10182,8 @@ int main(int argc, char *argv[]) {
      if(opt.summary)
          summary();

+    plugin_summary();
+
      if(opt.report_pcpu)
          report_pcpu();



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.