Refactoring with ABC and properties

hpcbench/api.py

@@ -3,20 +3,32 @@
 """
 
 import os.path as osp
-
 from six import with_metaclass
-
-from hpcbench.toolbox.class_lib import ClassRegistrar
+from abc import ABCMeta, abstractmethod, abstractproperty
+from collections import namedtuple
 
 __all__ = [
     'MetricsExtractor',
     'Benchmark',
 ]
 
+# Metrics have simply a unit and a type
+# namedtuples are compact and have a nice str representation
+Metric = namedtuple("Metric", "unit type")
+
+
+class Metrics:
+    """List of common metrics
+    """
+    Milisecond = Metric('ms', float)
+    Second = Metric('s', float)
 
-class MetricsExtractor(object):
+
+class MetricsExtractor(with_metaclass(ABCMeta, object)):
     """Extract data from a benchmark command outputs
     """
+
+    @abstractproperty
     def metrics(self):
         """List of exported metrics
 
@@ -35,6 +47,7 @@ def metrics(self):
         """
         raise NotImplementedError
 
+    @abstractmethod
     def extract(self, outdir, metas):
         """Extract metrics from benchmark output
 
@@ -68,26 +81,31 @@ def stderr(cls, outdir):
         return osp.join(outdir, 'sterrr.txt')
 
 
-class Benchmark(with_metaclass(ClassRegistrar, object)):
+class Benchmark(with_metaclass(ABCMeta, object)):
     """Declare benchmark utility
     """
 
-    name = None
+    # --- Class "static" properties ---
+    @abstractproperty
+    def name(self): pass
     """Get benchmark name
     :rtype: string
     """
 
-    description = None
+    @abstractproperty
+    def description(self): pass
     """Get benchmark long description
     :rtype: string
     """
+    # ---
 
     def __init__(self, attributes=None):
         self.attributes = attributes or {}
 
     def __str__(self):
         return self.name
 
+    @abstractproperty
     def execution_matrix(self):
         """Describe benchmark commands
 
@@ -136,12 +154,13 @@ def execution_matrix(self):
         """
         raise NotImplementedError
 
-    def pre_execution(self):
+    def pre_execute(self):
         """Method called before executing one of the command.
         Current working directory is the execution directory.
         """
         pass
 
+    @abstractproperty
     def metrics_extractors(self):
         """Describe how to extract metrics from files written by
         benchmark commands.
@@ -162,6 +181,7 @@ def metrics_extractors(self):
         """
         raise NotImplementedError
 
+    @abstractproperty
     def plots(self):
         """Describe figures to generate
 
@@ -184,3 +204,10 @@ def plots(self):
             callable object that will be given metrics to plot
         """
         raise NotImplementedError
+
+    @classmethod
+    def get_subclass(cls, name):
+        for subclass in cls.__subclasses__():
+            if subclass.name == name:
+                return subclass
+        raise NameError("Not a valid Benchmark class: " + name)

hpcbench/benchmark/sysbench.py

@@ -2,30 +2,37 @@
 
     https://github.com/akopytov/sysbench
 """
+from cached_property import cached_property
 import re
 
 from hpcbench.api import (
     Benchmark,
+    Metrics,
     MetricsExtractor,
 )
 
 
-KEEP_NUMBERS = re.compile('[^0-9.]')
-
-
-class cpu_extractor(MetricsExtractor):
+class CpuExtractor(MetricsExtractor):
     """Ignore stdout until this line"""
     STDOUT_IGNORE_PRIOR = 'Test execution summary:'
+    KEEP_NUMBERS = re.compile('[^0-9.]')
 
-    def metrics(self):
-        return dict(
-            minimum=dict(type=float, unit='ms'),
-            average=dict(type=float, unit='ms'),
-            maximum=dict(type=float, unit='ms'),
-            percentile95=dict(type=float, unit='ms'),
-            total_time=dict(type=float, unit='s'),
+    def __init__(self):
+        self._metrics = dict(
+            minimum=Metrics.Milisecond,
+            average=Metrics.Milisecond,
+            maximum=Metrics.Milisecond,
+            percentile95=Metrics.Milisecond,
+            total_time=Metrics.Second
         )
 
+    @property
+    def metrics(self):
+        """ The metrics to be extracted.
+            This property can not be replaced, but can be mutated as required
+        """
+        return self._metrics
+
     def extract(self, outdir, metas):
         mapping = {
             'min': 'minimum',
@@ -37,19 +44,16 @@ def extract(self, outdir, metas):
         metrics = {}
         # parse stdout and extract desired metrics
         with open(self.stdout(outdir)) as istr:
-            look_after_data = False
+            for line in istr:
+                if line.strip() == self.STDOUT_IGNORE_PRIOR:
+                    break
             for line in istr:
                 line = line.strip()
-                if not look_after_data:
-                    if line == self.STDOUT_IGNORE_PRIOR:
-                        look_after_data = True
-                        continue
-                else:
-                    for attr, metric in mapping.items():
-                        if line.startswith(attr + ':'):
-                            value = line[len(attr + ':'):].lstrip()
-                            value = KEEP_NUMBERS.sub('', value)
-                            metrics[metric] = float(value)
+                for attr, metric in mapping.items():
+                    if line.startswith(attr + ':'):
+                        value = line[len(attr + ':'):].lstrip()
+                        value = self.KEEP_NUMBERS.sub('', value)
+                        metrics[metric] = float(value)
         # ensure all metrics have been extracted
         unset_attributes = set(mapping.values()) - set(metrics)
         if any(unset_attributes):
@@ -74,6 +78,7 @@ def __init__(self):
         performance, and even MySQL benchmarking.
         """
 
+    @property
     def execution_matrix(self):
         if Sysbench.FEATURE_CPU in self.attributes['features']:
             for thread in [1, 4, 16]:
@@ -93,11 +98,13 @@ def execution_matrix(self):
                         )
                     )
 
+    @cached_property
     def metrics_extractors(self):
         return {
-            Sysbench.FEATURE_CPU: cpu_extractor(),
+            Sysbench.FEATURE_CPU: CpuExtractor(),
         }
 
+    @property
     def plots(self):
         return {
             Sysbench.FEATURE_CPU: [

hpcbench/driver.py

@@ -137,7 +137,7 @@ def __init__(self, campaign, name):
     def children(self):
         """Retrieve tags associated to the current node"""
         hostnames = {'localhost', self.name}
-        benchmarks = set(['*'])
+        benchmarks = {'*'}
         for tag, configs in self.campaign.network.tags.items():
             for config in configs:
                 for mode, kconfig in config.items():
@@ -188,7 +188,7 @@ def __init__(self, campaign, benchmark):
     @cached_property
     def children(self):
         categories = set()
-        for execution in self.benchmark.execution_matrix():
+        for execution in self.benchmark.execution_matrix:
             categories.add(execution['category'])
         return categories
 
@@ -206,7 +206,7 @@ def __init__(self, campaign, category, benchmark):
 
     @cached_property
     def plot_files(self):
-        for plot in self.benchmark.plots()[self.category]:
+        for plot in self.benchmark.plots[self.category]:
             yield osp.join(os.getcwd(), Plotter.get_filename(plot))
 
     @cached_property
@@ -219,7 +219,7 @@ def commands(self):
     @cached_property
     def children(self):
         children = []
-        for execution in self.benchmark.execution_matrix():
+        for execution in self.benchmark.execution_matrix:
             category = execution.get('category')
             if category != self.category:
                 continue
@@ -234,7 +234,7 @@ def children(self):
     def __call__(self, **kwargs):
         if "no_exec" not in kwargs:
             runs = dict()
-            for execution in self.benchmark.execution_matrix():
+            for execution in self.benchmark.execution_matrix:
                 category = execution.get('category')
                 if self.category != category:
                     continue
@@ -255,7 +255,7 @@ def __call__(self, **kwargs):
                     yield run_dir
             self.gather_metrics(runs)
         elif 'plot' in kwargs:
-            for plot in self.benchmark.plots().get(self.category):
+            for plot in self.benchmark.plots.get(self.category):
                 self.generate_plot(plot, self.category)
         else:
             runs = dict()
@@ -304,7 +304,8 @@ def generate_plot(self, desc, category):
 
 class MetricsDriver(object):
     """Abstract representation of metrics already
-    built by a previous run"""
+    built by a previous run
+    """
     def __init__(self, campaign, benchmark):
         self.campaign = campaign
         self.benchmark = benchmark
@@ -314,7 +315,7 @@ def __init__(self, campaign, benchmark):
     @write_yaml_report
     def __call__(self, **kwargs):
         cat = self.report.get('category')
-        all_extractors = self.benchmark.metrics_extractors()
+        all_extractors = self.benchmark.metrics_extractors
         if cat not in all_extractors:
             raise Exception('No extractor for benchmark category %s' %
                             cat)
@@ -330,14 +331,15 @@ def __call__(self, **kwargs):
         return self.report
 
     def check_metrics(self, extractor, metrics):
-        """Ensure that returned metrics are properly exposed"""
-        exposed_metrics = extractor.metrics()
+        """Ensure that returned metrics are properly exposed
+        """
+        exposed_metrics = extractor.metrics
         for name, value in metrics.items():
             metric = exposed_metrics.get(name)
             if not metric:
                 message = "Unexpected metric '{}' returned".format(name)
                 raise Exception(message)
-            elif not isinstance(value, metric['type']):
+            elif not isinstance(value, metric.type):
                 message = "Unexpected type for metrics {}".format(name)
                 raise Exception(message)
 
@@ -353,7 +355,7 @@ def __init__(self, campaign, benchmark, execution):
 
     @write_yaml_report
     def __call__(self, **kwargs):
-        self.benchmark.pre_execution()
+        self.benchmark.pre_execute()
         with open('stdout.txt', 'w') as stdout, \
                 open('stderr.txt', 'w') as stderr:
             kwargs = dict(stdout=stdout, stderr=stderr)