From ff366b3fd9d1a1d71186df78fc0ed8044a65d37c Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Mon, 21 Aug 2017 09:42:52 +0100 Subject: [PATCH 01/16] derived: renamed derived_measurments --> energy Renamed to reduce redandancy in import path. --- devlib/__init__.py | 2 +- devlib/derived/{derived_measurements.py => energy.py} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename devlib/derived/{derived_measurements.py => energy.py} (100%) diff --git a/devlib/__init__.py b/devlib/__init__.py index b1b4fa3..19232c7 100644 --- a/devlib/__init__.py +++ b/devlib/__init__.py @@ -20,7 +20,7 @@ from devlib.instrument.netstats import NetstatsInstrument from devlib.instrument.gem5power import Gem5PowerInstrument from devlib.derived import DerivedMeasurements -from devlib.derived.derived_measurements import DerivedEnergyMeasurements +from devlib.derived.energy import DerivedEnergyMeasurements from devlib.trace.ftrace import FtraceCollector diff --git a/devlib/derived/derived_measurements.py b/devlib/derived/energy.py similarity index 100% rename from devlib/derived/derived_measurements.py rename to devlib/derived/energy.py From dfd0b8ebd9a5f57aec6480ca6bd1d4d25763aedc Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Mon, 21 Aug 2017 11:01:42 +0100 Subject: [PATCH 02/16] MeasurementsCsv: rename itermeasurments Renamed to iter_measurments for readability. --- devlib/derived/energy.py | 2 +- devlib/instrument/__init__.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/devlib/derived/energy.py b/devlib/derived/energy.py index 770db88..b3f9c82 100644 --- a/devlib/derived/energy.py +++ b/devlib/derived/energy.py @@ -56,7 +56,7 @@ class DerivedEnergyMeasurements(DerivedMeasurements): power_results = defaultdict(float) # Process data - for count, row in enumerate(measurements_csv.itermeasurements()): + for count, row in enumerate(measurements_csv.iter_measurements()): if use_timestamp: last_ts = row_ts row_ts = time_measurment.convert(float(row[ts_index].value), 'time') diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index 77ba1d3..cceada6 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -152,9 +152,9 @@ class MeasurementsCsv(object): self._load_channels() def measurements(self): - return list(self.itermeasurements()) + return list(self.iter_measurements()) - def itermeasurements(self): + def iter_measurements(self): self._fh.seek(0) reader = csv.reader(self._fh) reader.next() # headings From 15333eb09c3d74491fd37b54fd53552ff0fc17b8 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Tue, 22 Aug 2017 09:27:24 +0100 Subject: [PATCH 03/16] utils/misc: update CPU_PART_MAP with Mongoose Add Samsung's Mongoose M1 core part identifiers to the map. --- devlib/utils/misc.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/devlib/utils/misc.py b/devlib/utils/misc.py index f601686..8cfd59f 100644 --- a/devlib/utils/misc.py +++ b/devlib/utils/misc.py @@ -103,6 +103,9 @@ CPU_PART_MAP = { 0x211: {0x1: 'KryoGold'}, 0x800: {None: 'Falkor'}, }, + 0x53: { # Samsung LSI + 0x001: {0x1: 'MongooseM1'}, + }, 0x56: { # Marvell 0x131: { 0x2: 'Feroceon 88F6281', From 2afa8f86a4faca3d4c7a5d80eae3b46086c5fc6f Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 30 Aug 2017 14:27:03 +0100 Subject: [PATCH 04/16] insturment: add catergory for time + doc fix - Add a category name for time MeasurmentType's, as there are now multiple. - Fix the names of time_ms and time_us in the documentation. --- devlib/instrument/__init__.py | 6 +++--- doc/instrumentation.rst | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index cceada6..6d11eda 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -75,19 +75,19 @@ class MeasurementType(object): # Standard measures _measurement_types = [ MeasurementType('unknown', None), - MeasurementType('time', 'seconds', + MeasurementType('time', 'seconds', 'time', conversions={ 'time_us': lambda x: x * 1000000, 'time_ms': lambda x: x * 1000, } ), - MeasurementType('time_us', 'microseconds', + MeasurementType('time_us', 'microseconds', 'time', conversions={ 'time': lambda x: x / 1000000, 'time_ms': lambda x: x / 1000, } ), - MeasurementType('time_ms', 'milliseconds', + MeasurementType('time_ms', 'milliseconds', 'time', conversions={ 'time': lambda x: x / 1000, 'time_us': lambda x: x * 1000, diff --git a/doc/instrumentation.rst b/doc/instrumentation.rst index 8aee1ce..6f381b4 100644 --- a/doc/instrumentation.rst +++ b/doc/instrumentation.rst @@ -229,11 +229,11 @@ defined measurement types are +-------------+-------------+---------------+ | name | units | category | +=============+=============+===============+ -| time | seconds | | +| time | seconds | time | +-------------+-------------+---------------+ -| time | microseconds| | +| time_us | microseconds| time | +-------------+-------------+---------------+ -| time | milliseconds| | +| time_ms | milliseconds| time | +-------------+-------------+---------------+ | temperature | degrees | | +-------------+-------------+---------------+ From 823ce718bf0edc6baa99c2184fe479186771988f Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 30 Aug 2017 14:55:42 +0100 Subject: [PATCH 05/16] instrument: add get_raw() API Derived metrics may be calculated form data in raw output that is not present in the resulting MeasurementCSV. This adds a method to provide uniform access to raw artifacts generated by an instrument. --- devlib/instrument/__init__.py | 3 +++ devlib/instrument/acmecape.py | 3 +++ devlib/instrument/daq.py | 6 ++++++ devlib/instrument/energy_probe.py | 11 ++++++++--- devlib/instrument/frames.py | 10 +++++++--- doc/instrumentation.rst | 11 +++++++++-- 6 files changed, 36 insertions(+), 8 deletions(-) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index 6d11eda..74113a3 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -297,3 +297,6 @@ class Instrument(object): def get_data(self, outfile): pass + + def get_raw(self): + return [] diff --git a/devlib/instrument/acmecape.py b/devlib/instrument/acmecape.py index e1bb6c1..1053c9d 100644 --- a/devlib/instrument/acmecape.py +++ b/devlib/instrument/acmecape.py @@ -121,3 +121,6 @@ class AcmeCapeInstrument(Instrument): output_row.append(float(row[i])/1000) writer.writerow(output_row) return MeasurementsCsv(outfile, self.active_channels, self.sample_rate_hz) + + def get_raw(self): + return [self.raw_data_file] diff --git a/devlib/instrument/daq.py b/devlib/instrument/daq.py index 75e854d..d497151 100644 --- a/devlib/instrument/daq.py +++ b/devlib/instrument/daq.py @@ -33,6 +33,7 @@ class DaqInstrument(Instrument): # pylint: disable=no-member super(DaqInstrument, self).__init__(target) self._need_reset = True + self._raw_files = [] if execute_command is None: raise HostError('Could not import "daqpower": {}'.format(import_error_mesg)) if labels is None: @@ -68,6 +69,7 @@ class DaqInstrument(Instrument): if not result.status == Status.OK: # pylint: disable=no-member raise HostError(result.message) self._need_reset = False + self._raw_files = [] def start(self): if self._need_reset: @@ -86,6 +88,7 @@ class DaqInstrument(Instrument): site = os.path.splitext(entry)[0] path = os.path.join(tempdir, entry) raw_file_map[site] = path + self._raw_files.append(path) active_sites = unique([c.site for c in self.active_channels]) file_handles = [] @@ -131,6 +134,9 @@ class DaqInstrument(Instrument): for fh in file_handles: fh.close() + def get_raw(self): + return self._raw_files + def teardown(self): self.execute('close') diff --git a/devlib/instrument/energy_probe.py b/devlib/instrument/energy_probe.py index 5f47430..c8f179e 100644 --- a/devlib/instrument/energy_probe.py +++ b/devlib/instrument/energy_probe.py @@ -52,6 +52,7 @@ class EnergyProbeInstrument(Instrument): self.raw_output_directory = None self.process = None self.sample_rate_hz = 10000 # Determined empirically + self.raw_data_file = None for label in self.labels: for kind in self.attributes: @@ -64,6 +65,7 @@ class EnergyProbeInstrument(Instrument): for i, rval in enumerate(self.resistor_values)] rstring = ''.join(parts) self.command = '{} -d {} -l {} {}'.format(self.caiman, self.device_entry, rstring, self.raw_output_directory) + self.raw_data_file = None def start(self): self.logger.debug(self.command) @@ -92,10 +94,10 @@ class EnergyProbeInstrument(Instrument): num_of_ports = len(self.resistor_values) struct_format = '{}I'.format(num_of_ports * self.attributes_per_sample) not_a_full_row_seen = False - raw_data_file = os.path.join(self.raw_output_directory, '0000000000') + self.raw_data_file = os.path.join(self.raw_output_directory, '0000000000') - self.logger.debug('Parsing raw data file: {}'.format(raw_data_file)) - with open(raw_data_file, 'rb') as bfile: + self.logger.debug('Parsing raw data file: {}'.format(self.raw_data_file)) + with open(self.raw_data_file, 'rb') as bfile: with open(outfile, 'wb') as wfh: writer = csv.writer(wfh) writer.writerow(active_channels) @@ -114,3 +116,6 @@ class EnergyProbeInstrument(Instrument): else: not_a_full_row_seen = True return MeasurementsCsv(outfile, self.active_channels, self.sample_rate_hz) + + def get_raw(self): + return [self.raw_data_file] diff --git a/devlib/instrument/frames.py b/devlib/instrument/frames.py index d1899fb..54869c1 100644 --- a/devlib/instrument/frames.py +++ b/devlib/instrument/frames.py @@ -20,6 +20,7 @@ class FramesInstrument(Instrument): self.collector = None self.header = None self._need_reset = True + self._raw_file = None self._init_channels() def reset(self, sites=None, kinds=None, channels=None): @@ -27,6 +28,7 @@ class FramesInstrument(Instrument): self.collector = self.collector_cls(self.target, self.period, self.collector_target, self.header) self._need_reset = False + self._raw_file = None def start(self): if self._need_reset: @@ -38,14 +40,16 @@ class FramesInstrument(Instrument): self._need_reset = True def get_data(self, outfile): - raw_outfile = None if self.keep_raw: - raw_outfile = outfile + '.raw' - self.collector.process_frames(raw_outfile) + self._raw_file = outfile + '.raw' + self.collector.process_frames(self._raw_file) active_sites = [chan.label for chan in self.active_channels] self.collector.write_frames(outfile, columns=active_sites) return MeasurementsCsv(outfile, self.active_channels, self.sample_rate_hz) + def get_raw(self): + return [self._raw_file] if self._raw_file else [] + def _init_channels(self): raise NotImplementedError() diff --git a/doc/instrumentation.rst b/doc/instrumentation.rst index 6f381b4..f23fc3e 100644 --- a/doc/instrumentation.rst +++ b/doc/instrumentation.rst @@ -65,8 +65,8 @@ Instrument :INSTANTANEOUS: The instrument supports taking a single sample via ``take_measurement()``. :CONTINUOUS: The instrument supports collecting measurements over a - period of time via ``start()``, ``stop()``, and - ``get_data()`` methods. + period of time via ``start()``, ``stop()``, ``get_data()``, + and (optionally) ``get_raw`` methods. .. note:: It's possible for one instrument to support more than a single mode. @@ -161,6 +161,13 @@ Instrument .. note:: This method is only implemented by :class:`Instrument`\ s that support ``CONTINUOUS`` measurement. +.. method:: Instrument.get_raw() + + Returns a list of paths to files containing raw output from the underlying + source(s) that is used to produce the data CSV. If now raw output is + generated or saved, an empty list will be returned. The format of the + contents of the raw files is entirely source-dependent. + .. attribute:: Instrument.sample_rate_hz Sample rate of the instrument in Hz. Assumed to be the same for all channels. From 9192deb8ee13a8105742631cece3b4be095c077f Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 31 Aug 2017 17:38:33 +0100 Subject: [PATCH 06/16] InstrumentChannel: name is now an alias for label In addition to a label constructed form the combination of site and measurment type, channels had a name that was specified on creation. This proven to be not particularly useful (there only being one instance of the name being set to something materially different from the label); and this has lead to channels being inconsistenly referenced (some times a channel is identified by its label, and sometimes by the name). This commit removes the name from __init__ arguments, and InstrumentChannel.name is now an alias for InstrumentChannel.label. --- devlib/instrument/__init__.py | 15 ++++++--------- devlib/instrument/hwmon.py | 2 +- devlib/platform/arm.py | 32 ++++++++++++++++---------------- 3 files changed, 23 insertions(+), 26 deletions(-) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index 74113a3..36bd4ed 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -175,14 +175,12 @@ class MeasurementsCsv(object): if entry.endswith(suffix): site = entry[:-len(suffix)] measure = mt - name = '{}_{}'.format(site, measure) break else: site = entry measure = 'unknown' - name = entry - chan = InstrumentChannel(name, site, measure) + chan = InstrumentChannel(site, measure) self.channels.append(chan) @@ -192,6 +190,8 @@ class InstrumentChannel(object): def label(self): return '{}_{}'.format(self.site, self.kind) + name = label + @property def kind(self): return self.measurement_type.name @@ -200,8 +200,7 @@ class InstrumentChannel(object): def units(self): return self.measurement_type.units - def __init__(self, name, site, measurement_type, **attrs): - self.name = name + def __init__(self, site, measurement_type, **attrs): self.site = site if isinstance(measurement_type, MeasurementType): self.measurement_type = measurement_type @@ -243,10 +242,8 @@ class Instrument(object): measure = measure.name return [c for c in self.list_channels() if c.kind == measure] - def add_channel(self, site, measure, name=None, **attrs): - if name is None: - name = '{}_{}'.format(site, measure) - chan = InstrumentChannel(name, site, measure, **attrs) + def add_channel(self, site, measure, **attrs): + chan = InstrumentChannel(site, measure, **attrs) self.channels[chan.label] = chan # initialization and teardown diff --git a/devlib/instrument/hwmon.py b/devlib/instrument/hwmon.py index ae49f40..5a9d8af 100644 --- a/devlib/instrument/hwmon.py +++ b/devlib/instrument/hwmon.py @@ -45,7 +45,7 @@ class HwmonInstrument(Instrument): measure = self.measure_map.get(ts.kind)[0] if measure: self.logger.debug('\tAdding sensor {}'.format(ts.name)) - self.add_channel(_guess_site(ts), measure, name=ts.name, sensor=ts) + self.add_channel(_guess_site(ts), measure, sensor=ts) else: self.logger.debug('\tSkipping sensor {} (unknown kind "{}")'.format(ts.name, ts.kind)) except ValueError: diff --git a/devlib/platform/arm.py b/devlib/platform/arm.py index e760eaf..76b58a4 100644 --- a/devlib/platform/arm.py +++ b/devlib/platform/arm.py @@ -210,22 +210,22 @@ class JunoEnergyInstrument(Instrument): mode = CONTINUOUS | INSTANTANEOUS _channels = [ - InstrumentChannel('sys_curr', 'sys', 'current'), - InstrumentChannel('a57_curr', 'a57', 'current'), - InstrumentChannel('a53_curr', 'a53', 'current'), - InstrumentChannel('gpu_curr', 'gpu', 'current'), - InstrumentChannel('sys_volt', 'sys', 'voltage'), - InstrumentChannel('a57_volt', 'a57', 'voltage'), - InstrumentChannel('a53_volt', 'a53', 'voltage'), - InstrumentChannel('gpu_volt', 'gpu', 'voltage'), - InstrumentChannel('sys_pow', 'sys', 'power'), - InstrumentChannel('a57_pow', 'a57', 'power'), - InstrumentChannel('a53_pow', 'a53', 'power'), - InstrumentChannel('gpu_pow', 'gpu', 'power'), - InstrumentChannel('sys_cenr', 'sys', 'energy'), - InstrumentChannel('a57_cenr', 'a57', 'energy'), - InstrumentChannel('a53_cenr', 'a53', 'energy'), - InstrumentChannel('gpu_cenr', 'gpu', 'energy'), + InstrumentChannel('sys', 'current'), + InstrumentChannel('a57', 'current'), + InstrumentChannel('a53', 'current'), + InstrumentChannel('gpu', 'current'), + InstrumentChannel('sys', 'voltage'), + InstrumentChannel('a57', 'voltage'), + InstrumentChannel('a53', 'voltage'), + InstrumentChannel('gpu', 'voltage'), + InstrumentChannel('sys', 'power'), + InstrumentChannel('a57', 'power'), + InstrumentChannel('a53', 'power'), + InstrumentChannel('gpu', 'power'), + InstrumentChannel('sys', 'energy'), + InstrumentChannel('a57', 'energy'), + InstrumentChannel('a53', 'energy'), + InstrumentChannel('gpu', 'energy'), ] def __init__(self, target): From 07ba177e58bfe5cd7e9a2788c519d0a173653fc9 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 7 Sep 2017 09:52:20 +0100 Subject: [PATCH 07/16] InstrumentChannel: allow None sites Allow site to be None (still must be explicitly specified). If the site is None, the label if created using only the measurement type. --- devlib/instrument/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index 36bd4ed..856a3af 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -188,7 +188,9 @@ class InstrumentChannel(object): @property def label(self): - return '{}_{}'.format(self.site, self.kind) + if self.site is not None: + return '{}_{}'.format(self.site, self.kind) + return self.kind name = label From 8479af48c46f76607ad6080daea7bc2b2e1863da Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 7 Sep 2017 10:13:59 +0100 Subject: [PATCH 08/16] MeasurementCsv: various enhancements - Added values() and iter_values() methods. These return each row as a named tuple, with channel labels as the field names. - __cmp__ has been made more generic by checking wether other has "value" attribute, rather than wether it is an instance of Measurment. - MeasurementCsv no longer keeps an open handle to the file, and instead re-opens the file each time it needs it. This removes the need for managing the open handle, and alows parallel iterations over the values (each iteration will have it's own read handle into the files). --- devlib/instrument/__init__.py | 41 +++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index 856a3af..99f4e5a 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -127,7 +127,7 @@ class Measurement(object): self.channel = channel def __cmp__(self, other): - if isinstance(other, Measurement): + if hasattr(other, 'value'): return cmp(self.value, other.value) else: return cmp(self.value, other) @@ -147,26 +147,32 @@ class MeasurementsCsv(object): self.path = path self.channels = channels self.sample_rate_hz = sample_rate_hz - self._fh = open(path, 'rb') if self.channels is None: self._load_channels() + headings = [chan.label for chan in self.channels] + self.data_tuple = collections.namedtuple('csv_entry', headings) def measurements(self): return list(self.iter_measurements()) def iter_measurements(self): - self._fh.seek(0) - reader = csv.reader(self._fh) - reader.next() # headings - for row in reader: + for row in self._iter_rows(): values = map(numeric, row) yield [Measurement(v, c) for (v, c) in zip(values, self.channels)] + def values(self): + return list(self.iter_values()) + + def iter_values(self): + for row in self._iter_rows(): + values = map(numeric, row) + yield self.data_tuple(*values) + def _load_channels(self): - self._fh.seek(0) - reader = csv.reader(self._fh) - header = reader.next() - self._fh.seek(0) + header = [] + with open(self.path, 'rb') as fh: + reader = csv.reader(fh) + header = reader.next() self.channels = [] for entry in header: @@ -177,12 +183,23 @@ class MeasurementsCsv(object): measure = mt break else: - site = entry - measure = 'unknown' + if entry in MEASUREMENT_TYPES: + site = None + measure = entry + else: + site = entry + measure = 'unknown' chan = InstrumentChannel(site, measure) self.channels.append(chan) + def _iter_rows(self): + with open(self.path, 'rb') as fh: + reader = csv.reader(fh) + reader.next() # headings + for row in reader: + yield row + class InstrumentChannel(object): From dd26b43ac5237d967dca702deb5274a566a8e885 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 7 Sep 2017 11:13:34 +0100 Subject: [PATCH 09/16] derived: DerivedMeasurments now return DerivedMetrics DerivedMeasurments processors now return DerviedMetrics rather than measurments. The notion of an InstrumentChannel doesn't really make sense in the context of DerivedMeasurments, which are not directly measured on the target. Since Measurement's require a channel, a simpler DerviedMetric is added that only requires a name and a type. --- devlib/__init__.py | 2 +- devlib/derived/__init__.py | 39 ++++++++++++++++++++++++++++++++++++ devlib/derived/energy.py | 12 +++++------ doc/derived_measurements.rst | 36 ++++++++++++++++++++++++++++++--- 4 files changed, 79 insertions(+), 10 deletions(-) diff --git a/devlib/__init__.py b/devlib/__init__.py index 19232c7..42509f9 100644 --- a/devlib/__init__.py +++ b/devlib/__init__.py @@ -19,7 +19,7 @@ from devlib.instrument.monsoon import MonsoonInstrument from devlib.instrument.netstats import NetstatsInstrument from devlib.instrument.gem5power import Gem5PowerInstrument -from devlib.derived import DerivedMeasurements +from devlib.derived import DerivedMeasurements, DerivedMetric from devlib.derived.energy import DerivedEnergyMeasurements from devlib.trace.ftrace import FtraceCollector diff --git a/devlib/derived/__init__.py b/devlib/derived/__init__.py index 5689a58..5f7dc6e 100644 --- a/devlib/derived/__init__.py +++ b/devlib/derived/__init__.py @@ -12,6 +12,45 @@ # See the License for the specific language governing permissions and # limitations under the License. # + +from devlib.instrument import MeasurementType, MEASUREMENT_TYPES + + +class DerivedMetric(object): + + __slots__ = ['name', 'value', 'measurement_type'] + + @property + def units(self): + return self.measurement_type.units + + def __init__(self, name, value, measurement_type): + self.name = name + self.value = value + if isinstance(measurement_type, MeasurementType): + self.measurement_type = measurement_type + else: + try: + self.measurement_type = MEASUREMENT_TYPES[measurement_type] + except KeyError: + msg = 'Unknown measurement type: {}' + raise ValueError(msg.format(measurement_type)) + + def __cmp__(self, other): + if hasattr(other, 'value'): + return cmp(self.value, other.value) + else: + return cmp(self.value, other) + + def __str__(self): + if self.units: + return '{}: {} {}'.format(self.name, self.value, self.units) + else: + return '{}: {}'.format(self.name, self.value) + + __repr__ = __str__ + + class DerivedMeasurements(object): @staticmethod diff --git a/devlib/derived/energy.py b/devlib/derived/energy.py index b3f9c82..84d3d7c 100644 --- a/devlib/derived/energy.py +++ b/devlib/derived/energy.py @@ -15,8 +15,8 @@ from __future__ import division from collections import defaultdict -from devlib import DerivedMeasurements -from devlib.instrument import Measurement, MEASUREMENT_TYPES, InstrumentChannel +from devlib import DerivedMeasurements, DerivedMetric +from devlib.instrument import MEASUREMENT_TYPES, InstrumentChannel class DerivedEnergyMeasurements(DerivedMeasurements): @@ -86,12 +86,12 @@ class DerivedEnergyMeasurements(DerivedMeasurements): derived_measurements = [] for site in energy_results: total_energy = energy_results[site]['end'] - energy_results[site]['start'] - instChannel = InstrumentChannel('cum_energy', site, MEASUREMENT_TYPES['energy']) - derived_measurements.append(Measurement(total_energy, instChannel)) + name = '{}_total_energy'.format(site) + derived_measurements.append(DerivedMetric(name, total_energy, MEASUREMENT_TYPES['energy'])) for site in power_results: power = power_results[site] / (count + 1) #pylint: disable=undefined-loop-variable - instChannel = InstrumentChannel('avg_power', site, MEASUREMENT_TYPES['power']) - derived_measurements.append(Measurement(power, instChannel)) + name = '{}_average_power'.format(site) + derived_measurements.append(DerivedMetric(name, power, MEASUREMENT_TYPES['power'])) return derived_measurements diff --git a/doc/derived_measurements.rst b/doc/derived_measurements.rst index fcd497c..02a6daa 100644 --- a/doc/derived_measurements.rst +++ b/doc/derived_measurements.rst @@ -9,7 +9,7 @@ Example ------- The following example shows how to use an implementation of a -:class:`DerivedMeasurement` to obtain a list of calculated ``Measurements``. +:class:`DerivedMeasurement` to obtain a list of calculated ``DerivedMetric``'s. .. code-block:: ipython @@ -42,9 +42,39 @@ Derived Measurements .. method:: DerivedMeasurements.process(measurement_csv) - Returns a list of :class:`Measurement` objects that have been calculated. + Returns a list of :class:`DerivedMetric` objects that have been calculated. +Derived Metric +~~~~~~~~~~~~~~ + +.. class:: DerivedMetric + + Represents a metric derived from previously collected ``Measurement``s. + Unlike, a ``Measurement``, this was not measured directly from the target. + + +.. attribute:: DerivedMetric.name + + The name of the derived metric. This uniquely defines a metric -- two + ``DerivedMetric`` objects with the same ``name`` represent to instances of + the same metric (e.g. computed from two different inputs). + +.. attribute:: DerivedMetric.value + + The ``numeric`` value of the metric that has been computed for a particular + input. + +.. attribute:: DerivedMetric.measurement_type + + The ``MeasurementType`` of the metric. This indicates which conceptual + category the metric falls into, its units, and conversions to other + measurement types. + +.. attribute:: DerivedMetric.units + + The units in which the metric's value is expressed. + Available Derived Measurements ------------------------------- @@ -63,7 +93,7 @@ Available Derived Measurements .. method:: DerivedEnergyMeasurements.process(measurement_csv) - Returns a list of :class:`Measurement` objects that have been calculated for + Returns a list of :class:`DerivedMetric` objects that have been calculated for the average power and cumulative energy for each site. From adf25f93bb34eb8c293f80dc1ce3f82022dccfca Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 7 Sep 2017 14:19:24 +0100 Subject: [PATCH 10/16] DerivedMeasurements: add process_raw() + doc updates - Add process_raw() method to the API. This is inteneded to be invoked on any raw output (i.e. not MeasurmentCsv) generated by an Instrument. - Both process() process_raw() are portional to be overriden by impolementation; the default behavior is to return an empty list. - The output specification for both is extened to allow MeasurementCsv's, as well as DerivedMetric's. - Documentation has been reworded for clarity. --- devlib/derived/__init__.py | 8 +++-- doc/derived_measurements.rst | 59 ++++++++++++++++++++++++++---------- 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/devlib/derived/__init__.py b/devlib/derived/__init__.py index 5f7dc6e..24ac060 100644 --- a/devlib/derived/__init__.py +++ b/devlib/derived/__init__.py @@ -53,6 +53,8 @@ class DerivedMetric(object): class DerivedMeasurements(object): - @staticmethod - def process(measurements_csv): - raise NotImplementedError() + def process(self, measurements_csv): + return [] + + def process_raw(self, *args): + return [] diff --git a/doc/derived_measurements.rst b/doc/derived_measurements.rst index 02a6daa..285bce6 100644 --- a/doc/derived_measurements.rst +++ b/doc/derived_measurements.rst @@ -35,14 +35,29 @@ API Derived Measurements ~~~~~~~~~~~~~~~~~~~~ -.. class:: DerivedMeasurements() +.. class:: DerivedMeasurements - The ``DerivedMeasurements`` class is an abstract base for implementing - additional classes to calculate various metrics. + The ``DerivedMeasurements`` class provides an API for post-processing + instrument output offline (i.e. without a connection to the target device) to + generate additional metrics. .. method:: DerivedMeasurements.process(measurement_csv) - Returns a list of :class:`DerivedMetric` objects that have been calculated. + Process a :class:`MeasurementsCsv`, returning a list of + :class:`DerivedMetric` and/or :class:`MeasurementsCsv` objects that have been + derived from the input. The exact nature and ordering of the list memebers + is specific to indivial 'class'`DerivedMeasurements` implementations. + +.. method:: DerivedMeasurements.process_raw(\*args) + + Process raw output from an instrument, returnin a list :class:`DerivedMetric` + and/or :class:`MeasurementsCsv` objects that have been derived from the + input. The exact nature and ordering of the list memebers is specific to + indivial 'class'`DerivedMeasurements` implewmentations. + + The arguents to this method should be paths to raw output files generated by + an instrument. The number and order of expected arguments is specific to + particular implmentations. Derived Metric @@ -78,22 +93,34 @@ Derived Metric Available Derived Measurements ------------------------------- -.. class:: DerivedEnergyMeasurements() - The ``DerivedEnergyMeasurements`` class is used to calculate average power and - cumulative energy for each site if the required data is present. +.. note:: If a method of the API is not documented for a particular + implementation, that means that it s not overriden by that + implementation. It is still safe to call it -- an empty list will be + returned. - The calculation of cumulative energy can occur in 3 ways. If a - ``site`` contains ``energy`` results, the first and last measurements are extracted - and the delta calculated. If not, a ``timestamp`` channel will be used to calculate - the energy from the power channel, failing back to using the sample rate attribute - of the :class:`MeasurementCsv` file if timestamps are not available. If neither - timestamps or a sample rate are available then an error will be raised. +Energy +~~~~~~ + +.. class:: DerivedEnergyMeasurements + + The ``DerivedEnergyMeasurements`` class is used to calculate average power and + cumulative energy for each site if the required data is present. + + The calculation of cumulative energy can occur in 3 ways. If a + ``site`` contains ``energy`` results, the first and last measurements are extracted + and the delta calculated. If not, a ``timestamp`` channel will be used to calculate + the energy from the power channel, failing back to using the sample rate attribute + of the :class:`MeasurementCsv` file if timestamps are not available. If neither + timestamps or a sample rate are available then an error will be raised. .. method:: DerivedEnergyMeasurements.process(measurement_csv) - Returns a list of :class:`DerivedMetric` objects that have been calculated for - the average power and cumulative energy for each site. - + This will return total cumulative energy for each energy channel, and the + average power for each power channel in the input CSV. The output will contain + all energy metrics followed by power metrics. The ordering of both will match + the ordering of channels in the input. The metrics will by named based on the + sites of the coresponding channels according to the following patters: + ``"_total_energy"`` and ``"_average_power"``. From 01b5cffe03b68bb5cb74c4f344b0552e7cb75309 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 7 Sep 2017 14:26:04 +0100 Subject: [PATCH 11/16] instrument: Update MeasurementType table - Add generic "count" and "percent" MeasurementType's. - Add "fps" MeasurementType. - Add "thermal" category for "termperature" - Add a comment describing each category --- devlib/instrument/__init__.py | 26 ++++++++++++++++++++++++-- doc/instrumentation.rst | 6 ++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/devlib/instrument/__init__.py b/devlib/instrument/__init__.py index 99f4e5a..0d2c1ed 100644 --- a/devlib/instrument/__init__.py +++ b/devlib/instrument/__init__.py @@ -72,9 +72,25 @@ class MeasurementType(object): return text.format(self.name, self.units) -# Standard measures +# Standard measures. In order to make sure that downstream data processing is not tied +# to particular insturments (e.g. a particular method of mearuing power), instruments +# must, where possible, resport their measurments formatted as on of the standard types +# defined here. _measurement_types = [ + # For whatever reason, the type of measurement could not be established. MeasurementType('unknown', None), + + # Generic measurements + MeasurementType('count', 'count'), + MeasurementType('percent', 'percent'), + + # Time measurement. While there is typically a single "canonical" unit + # used for each type of measurmenent, time may be measured to a wide variety + # of events occuring at a wide range of scales. Forcing everying into a + # single scale will lead to inefficient and awkward to work with result tables. + # Coversion functions between the formats are specified, so that downstream + # processors that expect all times time be at a particular scale can automatically + # covert without being familar with individual instruments. MeasurementType('time', 'seconds', 'time', conversions={ 'time_us': lambda x: x * 1000000, @@ -93,17 +109,23 @@ _measurement_types = [ 'time_us': lambda x: x * 1000, } ), - MeasurementType('temperature', 'degrees'), + # Measurements related to thermals. + MeasurementType('temperature', 'degrees', 'thermal'), + + # Measurements related to power end energy consumption. MeasurementType('power', 'watts', 'power/energy'), MeasurementType('voltage', 'volts', 'power/energy'), MeasurementType('current', 'amps', 'power/energy'), MeasurementType('energy', 'joules', 'power/energy'), + # Measurments realted to data transfer, e.g. neworking, + # memory, or backing storage. MeasurementType('tx', 'bytes', 'data transfer'), MeasurementType('rx', 'bytes', 'data transfer'), MeasurementType('tx/rx', 'bytes', 'data transfer'), + MeasurementType('fps', 'fps', 'ui render'), MeasurementType('frames', 'frames', 'ui render'), ] for m in _measurement_types: diff --git a/doc/instrumentation.rst b/doc/instrumentation.rst index f23fc3e..0d4a6ce 100644 --- a/doc/instrumentation.rst +++ b/doc/instrumentation.rst @@ -236,13 +236,15 @@ defined measurement types are +-------------+-------------+---------------+ | name | units | category | +=============+=============+===============+ -| time | seconds | time | +| count | count | | ++-------------+-------------+---------------+ +| percent | percent | | +-------------+-------------+---------------+ | time_us | microseconds| time | +-------------+-------------+---------------+ | time_ms | milliseconds| time | +-------------+-------------+---------------+ -| temperature | degrees | | +| temperature | degrees | thermal | +-------------+-------------+---------------+ | power | watts | power/energy | +-------------+-------------+---------------+ From a8ca0fc6c834a2e3e145e6d102fff9133cee2db6 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Thu, 7 Sep 2017 14:34:55 +0100 Subject: [PATCH 12/16] util/rendering: add gfxinfo_get_last_dump Add gfxinfo_get_last_dump utility function to get the last gfxinfo dump from a (potentially large) file containing a concatenation of such dumps (as in the raw output of the GfxinfoFrames instrument). --- devlib/utils/rendering.py | 40 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/devlib/utils/rendering.py b/devlib/utils/rendering.py index 3b7b6c4..9ab1e00 100644 --- a/devlib/utils/rendering.py +++ b/devlib/utils/rendering.py @@ -203,3 +203,43 @@ class GfxinfoFrameCollector(FrameCollector): if not found: logger.warning('Could not find frames data in gfxinfo output') return + + +def _file_reverse_iter(fh, buf_size=1024): + fh.seek(0, os.SEEK_END) + offset = 0 + file_size = remaining_size = fh.tell() + while remaining_size > 0: + offset = min(file_size, offset + buf_size) + fh.seek(file_size - offset) + buf = fh.read(min(remaining_size, buf_size)) + remaining_size -= buf_size + yield buf + + +def gfxinfo_get_last_dump(filepath): + """ + Return the last gfxinfo dump from the frame collector's raw output. + + """ + record = '' + with open(filepath, 'r') as fh: + fh_iter = _file_reverse_iter(fh) + try: + while True: + buf = fh_iter.next() + ix = buf.find('** Graphics') + if ix >= 0: + return buf[ix:] + record + + ix = buf.find(' **\n') + if ix >= 0: + buf = fh_iter.next() + buf + ix = buf.find('** Graphics') + if ix < 0: + msg = '"{}" appears to be corrupted' + raise RuntimeError(msg.format(filepath)) + return buf[ix:] + record + record = buf + record + except StopIteration: + pass From 50dfb297cd1dfc2082a60a25e422796869b97186 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Wed, 13 Sep 2017 11:36:41 +0100 Subject: [PATCH 13/16] utils/rendering: fix surfaceflinger list SurfaceFlingerFrameCollector.list now converts line endings before splitting, so it now works when the endings are something other than "\r\n". --- devlib/utils/rendering.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/devlib/utils/rendering.py b/devlib/utils/rendering.py index 9ab1e00..a273cc7 100644 --- a/devlib/utils/rendering.py +++ b/devlib/utils/rendering.py @@ -122,7 +122,8 @@ class SurfaceFlingerFrameCollector(FrameCollector): return self.target.execute(cmd.format(activity)) def list(self): - return self.target.execute('dumpsys SurfaceFlinger --list').split('\r\n') + text = self.target.execute('dumpsys SurfaceFlinger --list') + return text.replace('\r\n', '\n').replace('\r', '\n').split('\n') def _process_raw_file(self, fh): text = fh.read().replace('\r\n', '\n').replace('\r', '\n') From d952abf52e4fbe1688b279a69ba3dee43897ed67 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Tue, 19 Sep 2017 13:48:08 +0100 Subject: [PATCH 14/16] utils/rendering: frame collectors should respect column order Previously FrameCollector.write_frames used "columns" argument only as a filter for which columns to write, but the order would always be the same as in raw output. The Instrument API requires that the column ordering in the resulting MeasurementsCsv matches the ordering of channels specified in reset() (if any). This means the collectors should respect the ordering specified in the "columns" parameter (which gets populated based on active channels). --- devlib/utils/rendering.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/devlib/utils/rendering.py b/devlib/utils/rendering.py index a273cc7..665135a 100644 --- a/devlib/utils/rendering.py +++ b/devlib/utils/rendering.py @@ -83,9 +83,14 @@ class FrameCollector(threading.Thread): header = self.header frames = self.frames else: - header = [c for c in self.header if c in columns] - indexes = [self.header.index(c) for c in header] + indexes = [] + for c in columns: + if c not in self.header: + msg = 'Invalid column "{}"; must be in {}' + raise ValueError(msg.format(c, self.header)) + indexes.append(self.header.index(c)) frames = [[f[i] for i in indexes] for f in self.frames] + header = columns with open(outfile, 'w') as wfh: writer = csv.writer(wfh) if header: From 96693a30354955b0037ba128bb0cf152e71020dd Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Fri, 22 Sep 2017 17:39:17 +0100 Subject: [PATCH 15/16] AndroidTarget: fix get_pid_of for recent Androids ps on recent Androids no longer takes an optional comm name; use Target.ps() instead, and filter by process name on the host. --- devlib/target.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/devlib/target.py b/devlib/target.py index 51826fb..fca798c 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -1011,11 +1011,12 @@ class AndroidTarget(Target): self.uninstall_executable(name) def get_pids_of(self, process_name): - result = self.execute('ps {}'.format(process_name[-15:]), check_exit_code=False).strip() - if result and 'not found' not in result: - return [int(x.split()[1]) for x in result.split('\n')[1:]] - else: - return [] + result = [] + search_term = process_name[-15:] + for entry in self.ps(): + if search_term in entry.name: + result.append(entry.pid) + return result def ps(self, **kwargs): lines = iter(convert_new_lines(self.execute('ps')).split('\n')) From 109fcc6deb79606faf197b58a131e977355afec6 Mon Sep 17 00:00:00 2001 From: Sergei Trofimov Date: Tue, 26 Sep 2017 13:30:15 +0100 Subject: [PATCH 16/16] AndroidTarget: fix ps() ps() splits the output on whiestspace into fields; always expecting nine. In some cases, wchan field may be blank, resulting in only eight chunks after the split. Detect that case and insert and empty entry at the appropriate index. --- devlib/target.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/devlib/target.py b/devlib/target.py index fca798c..4b2da42 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -1024,8 +1024,12 @@ class AndroidTarget(Target): result = [] for line in lines: parts = line.split(None, 8) - if parts: - result.append(PsEntry(*(parts[0:1] + map(int, parts[1:5]) + parts[5:]))) + if not parts: + continue + if len(parts) == 8: + # wchan was blank; insert an empty field where it should be. + parts.insert(5, '') + result.append(PsEntry(*(parts[0:1] + map(int, parts[1:5]) + parts[5:]))) if not kwargs: return result else: