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: Ensure timestamp uniqueness #295

Closed douglas-raillard-arm closed 4 years ago

douglas-raillard-arm commented 5 years ago

The timestamp of consecutive events are sometimes the same, if they were emitted in a very short interval of time. This becomes an issue when the events are dispatched in separate dataframes, which then sometimes need to be joined back together in client code. At this point, the ordering between events can be lost, leading to wrong results when exact ordering is required.

In order to fix that, pick the next representable floating value when there is an ambiguity. This ensures that the delta added is the smallest possible, and is typically in order of 1e-11.

douglas-raillard-arm commented 5 years ago

Updated with correction on the minimal increment description.

Since https://github.com/ARM-software/lisa/pull/1011 depends on it, I'll merge in 2 weeks if there is no comment (case in which something would break, or alternative implementation).

douglas-raillard-arm commented 5 years ago

wants to actually see the duplicate value

Duplicates in Trappy are currently very unlikely to be true duplicates, since AFAIK the current precision is microseconds: Quoting man trace-cmd-report:

But the default display of the  timestamp is only to the microsecond.

That PR changes the default to -t, which should enable true nanosecond timestamps.

Also, there is no guarantee at CPU level that an equal nanosecond timestamp means the events happened at the same time. Some clocks may increment by more than 1 the counter to save energy (see Generic Timer documentation in the ARM ARM). In other words, different timestamp mean an ordering between events in some circumstances, but an equal timestamp can never be assumed to mean that the events happened at perfectly the same time. All what this PR does is preserve ordering when there is one, at the cost of an amount of noise lower than what existing code needs to already cope with.

douglas-raillard-arm commented 5 years ago

but if we can add an extra column of the original timestamp so that if someone really wants them they can find them that'd be great

It could be interesting to keep an integer column with the number as coming from trace-cmd, without any floating conversion noise or extra noise such as the one introduced by this PR. We can add it when a test actually need that I guess.