This is an archive of the discontinued LLVM Phabricator instance.

[dsymutil] Replace TimeValue with TimePoint
ClosedPublic

Authored by labath on Oct 24 2016, 8:35 AM.

Details

Summary

All changes are pretty straight-forward. I chose to use TimePoints with
second precision, as that is all that seems to be required here.

One thing I noticed when writing this is that constructing a "zero" time point is
much more laborious than it used to be with TimeValues. Potentially, we may want
to consider adding something akin to TimeValue::PosixZeroTime() to time points as
well.

Event Timeline

labath updated this revision to Diff 75584.Oct 24 2016, 8:35 AM
labath retitled this revision from to [dsymutil] Replace TimeValue with TimePoint.
labath updated this object.
labath added reviewers: friss, zturner.
labath added a subscriber: llvm-commits.
friss edited edge metadata.Oct 24 2016, 8:42 AM

Pardon my ignorance, but what's the upside of TimePoints one TimeValues?

The main advantage is that it's a standard solution (TimePoint is just a typedef for std::chrono::time_point<std::chrono::system_clock, ...>), whereas TimeValue is our own hand-rolled implementation, which was added before std::chrono existed.

Apart from that, it gives you type safety wrt. durations and time points (modification time is a time point, a "timeout" value is a duration, the difference of two time points is a duration, etc.) and ability to safely work with and combine values with different precisions (1 second + 10 milliseconds == 1010 milliseconds).

labath added a comment.Nov 8 2016, 3:20 AM

If you don't have any issues with this, I'd like to commit this tomorrow (this is one of the last usages of TimeValue now).

This revision was automatically updated to reflect the committed changes.