Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal]: Stopwatch.GetElapsedTime #65858

Closed
stephentoub opened this issue Feb 24, 2022 · 15 comments · Fixed by #66372
Closed

[API Proposal]: Stopwatch.GetElapsedTime #65858

stephentoub opened this issue Feb 24, 2022 · 15 comments · Fixed by #66372
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Diagnostics enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@stephentoub
Copy link
Member

stephentoub commented Feb 24, 2022

Background and motivation

Stopwatch is a class and allocates, but when used for precision timing, you often don't want the overhead of that allocation, and it can be ergonomically inconvenient to try to cache an instance away somewhere. Developers that want to avoid that allocation end up using Stopwatch.GetTimestamp() directly, at which point they're having to remember what math to do and repeatedly do it for computing a TimeSpan from two timestamps. We can expose a helper to make that easier.

API Proposal

namespace System.Diagnostics
{
    public class Stopwatch
    {
+        public static TimeSpan GetElapsedTime(long startingTimestamp) => GetElapsedTime(startTimestamp, Stopwatch.GetTimestamp());
+        public static TimeSpan GetElapsedTime(long startingTimestamp, long endingTimestamp) => new TimeSpan((long) ((endTimestamp - startTimestamp) * s_tickFrequency) );
    }
}

(based on the discussion in #48570 (comment))

API Usage

long starting = Stopwatch.GetTimestamp();
...
long ending = Stopwatch.GetTimestamp();
...
TimeSpan time = Stopwatch.GetElapsedTime(starting, ending);

or

long starting = Stopwatch.GetTimestamp();
...
TimeSpan time = Stopwatch.GetElapsedTime(starting);

Alternative Designs

#48570

Risks

No response

@stephentoub stephentoub added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Feb 24, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Feb 24, 2022
@ghost
Copy link

ghost commented Feb 24, 2022

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Stopwatch is a class and allocates, but when used for precision timing, you often don't want the overhead of that allocation, and it can be ergonomically inconvenient to try to cache an instance away somewhere. Developers that want to avoid that allocation end up using Stopwatch.GetTimestamp() directly, at which point they're having to remember what math to do and repeatedly do it for computing a TimeSpan from two timestamps. We can expose a helper to make that easier.

API Proposal

namespace System.Diagnostics
{
    public class Stopwatch
    {
+        public static TimeSpan GetElapsedTime(long startingTimestamp) => GetElapsedTime(startTimestamp, Stopwatch.GetTimestamp());
+        public static TimeSpan GetElapsedTime(long startingTimestamp, long endingTimestamp) => new TimeSpan((endTimestamp - startTimestamp) * s_tickFrequency);
    }
}

(based on the discussion in #48570 (comment))

API Usage

long starting = Stopwatch.GetTimestamp();
...
long ending = Stopwatch.GetTimestamp();
...
TimeSpan time = Stopwatch.GetElapsedTime(starting, ending);

or

long starting = Stopwatch.GetTimestamp();
...
TimeSpan time = Stopwatch.GetElapsedTime(starting);

Alternative Designs

#48570

Risks

No response

Author: stephentoub
Assignees: -
Labels:

api-suggestion, area-System.Runtime, untriaged

Milestone: -

@skyoxZ
Copy link
Contributor

skyoxZ commented Feb 25, 2022

public static TimeSpan GetElapsedTime(long startingTimestamp, long endingTimestamp) 
    => new TimeSpan((endTimestamp - startTimestamp) * s_tickFrequency);

what happens when overflow?

@Symbai
Copy link

Symbai commented Feb 25, 2022

A new API for a one liner... I've seen a lot of APIs being rejected because of that. And they were for far more popular use cases than this.

@stephentoub
Copy link
Member Author

stephentoub commented Feb 25, 2022

A new API for a one liner... I've seen a lot of APIs being rejected because of that. And they were for far more popular use cases than this

You can write very complicated code in "one line". The key aspect here is how hard is it to determine what the code should be and how hard is it to understand/read and determine its correctness. Note that the actual code someone external to Stopwatch would have to write is more like this:

new TimeSpan((long)((Stopwatch.GetTimestamp() - startTimestamp) * (TimeSpan.TicksPerSecond / Stopwatch.Frequency)))

I'm very experienced with .NET, but every time I need to come up with that "line" of code, I need to both consult the documentation and think hard about whether I've got it right. It is very non-obvious (in part because you need to understand the different units between the variable frequency of stopwatch and the 100-nanosecond tick unit of TimeSpan), which makes it a fine candidate for a new method. And it's more commonly needed than your comment suggests.

what happens when overflow?

Which overflow? Do you mean if the value returned from GetTimestamp() fully wraps around the Int64 space? That's not new. This is the same logic employed by Stopwatch.Elapsed.

@skyoxZ
Copy link
Contributor

skyoxZ commented Feb 25, 2022

@stephentoub

what happens when overflow?

Should we check if ending - starting overflow? Stopwatch.GetElapsedTime expects that starting and ending are from GetTimestamp() so it's always safe, but actually the arguments can be any value.

@stephentoub
Copy link
Member Author

Should we check if ending - starting overflow?

Ah. We could. For valid inputs it would take 29,000 years for it to actually overflow. But, sure, if we wanted to flag misuse, we could add a check that (ulong)end - (ulong)start was within bounds. My personal preference would be not to spend any additional cycles on such a check, but I don't feel strongly about it.

@tannergooding
Copy link
Member

My personal preference would be not to spend any additional cycles on such a check, but I don't feel strongly about it.

My stance is the same. There is a preference for just doing the natural thing and that users would quickly realize the error point.

This is more of a primitive building block and ideally something like Stopwatch itself could use these helpers without risking a regression.

@ghost
Copy link

ghost commented Feb 25, 2022

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Stopwatch is a class and allocates, but when used for precision timing, you often don't want the overhead of that allocation, and it can be ergonomically inconvenient to try to cache an instance away somewhere. Developers that want to avoid that allocation end up using Stopwatch.GetTimestamp() directly, at which point they're having to remember what math to do and repeatedly do it for computing a TimeSpan from two timestamps. We can expose a helper to make that easier.

API Proposal

namespace System.Diagnostics
{
    public class Stopwatch
    {
+        public static TimeSpan GetElapsedTime(long startingTimestamp) => GetElapsedTime(startTimestamp, Stopwatch.GetTimestamp());
+        public static TimeSpan GetElapsedTime(long startingTimestamp, long endingTimestamp) => new TimeSpan((long) ((endTimestamp - startTimestamp) * s_tickFrequency) );
    }
}

(based on the discussion in #48570 (comment))

API Usage

long starting = Stopwatch.GetTimestamp();
...
long ending = Stopwatch.GetTimestamp();
...
TimeSpan time = Stopwatch.GetElapsedTime(starting, ending);

or

long starting = Stopwatch.GetTimestamp();
...
TimeSpan time = Stopwatch.GetElapsedTime(starting);

Alternative Designs

#48570

Risks

No response

Author: stephentoub
Assignees: -
Labels:

api-suggestion, area-System.Diagnostics, untriaged

Milestone: -

@tannergooding
Copy link
Member

Notably, I think this is also area-System.Diagnostics and not area-System.Runtime, so I need to remark this and the area owners there would need to mark api-ready-for-review

@tommcdon tommcdon added this to the 7.0.0 milestone Feb 25, 2022
@tommcdon tommcdon added enhancement Product code improvement that does NOT require public API changes/additions and removed untriaged New issue has not been triaged by the area owner labels Feb 25, 2022
@stephentoub stephentoub self-assigned this Feb 28, 2022
@stephentoub stephentoub added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Feb 28, 2022
@GSPP
Copy link

GSPP commented Feb 28, 2022

An alternative would be the proposed MonotonicTime struct from:

This would free the user from thinking about timestamps and frequencies. A MonotonicTime is just a timestamp wrapped in a convenience type.

var start = MonotonicTime.Current;

TimedAction();

var elapsed = MonotonicTime.Current - start;

@stephentoub
Copy link
Member Author

An alternative

Such a type would still need this calculation, so I don't see it as an alternative; if such a thing were to be added, it would be built on top of this building block.

Fundamentally Stopwatch today exposes a method to get a timestamp but then nothing to use it / translate it into a meaningful value... this fills that gap.

@HobbsCode
Copy link

There is a comment within Stopwatch.Stop() that appears to be relevant. Maybe a check for negative is necessary.

 // When measuring small time periods the Stopwatch.Elapsed*
 // properties can return negative values.  This is due to
 // bugs in the basic input/output system (BIOS) or the hardware
 // abstraction layer (HAL) on machines with variable-speed CPUs
 // (e.g. Intel SpeedStep).

@skyoxZ
Copy link
Contributor

skyoxZ commented Mar 1, 2022

Negative is ok, I think. Stopwatch.GetElapsedTime may be used to calculate the time difference between two events which it's not guaranteed which one happened first.

@terrajobst
Copy link
Member

terrajobst commented Mar 8, 2022

Video

  • Looks good as proposed
namespace System.Diagnostics
{
    public partial class Stopwatch
    {
        public static TimeSpan GetElapsedTime(long startingTimestamp);
        public static TimeSpan GetElapsedTime(long startingTimestamp, long endingTimestamp);
    }
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Mar 8, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Mar 9, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 9, 2022
@dotnet dotnet locked as resolved and limited conversation to collaborators Apr 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Diagnostics enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants