ARM-software / trappy

This repository has moved to https://gitlab.arm.com/tooling/trappy
Apache License 2.0
60 stars 39 forks source link

ftrace: Homogenize timestamps conversions #273

Closed valschneider closed 6 years ago

valschneider commented 6 years ago

By default, the str to float conversion done when reading from csv is different from the one used when reading from the trace.txt file.

Here's an example:

To fix this, the timestamps read from the cache are rounded to microsecond precision, which results in cache-read timestamps being identical to trace-read timestamps.

Tests have also been added/tweaked to ensure this stays true.

valschneider commented 6 years ago

While swimming around the code I found that comment:

# The timestamp, depending on the trace_clock configuration, can be
# reported either in [s].[us] or [ns] format. Let's ensure that we
# always generate DF which have the index expressed in:
#    [s].[decimals]

The [ns] format could potentially break things, but we don't have any trace to test this. Is this configuration actually common, or will we always (~99% of the time) be happy with [s].[us] ?

derkling commented 6 years ago

Mmm... I think I probably added that comment, isn't it? However, it happened sometimes in the past that, depending on how you configure the clock source, you can have traces with [ns] timestamps... but AFAIR, that comment should come with a patch which goal was to ensure that we always store the index in [s].[us] format, isn't it?

valschneider commented 6 years ago

Yes indeed, I was mostly thinking it could be worth adding a test for this scenario at some point.

valschneider commented 6 years ago

As @bjackman aptly pointed out, the origin of the problem is simply that the CSV lookup uses a different floating conversion method as the trace.txt parser. This can be fixed in a much cleaner way by simply forcing read_csv() to use python's float() conversion method instead of panda/numpy's. This revised commit does just that.