System.Diagnostics.Stopwatch When measuring small time periods

[andreas-synnerdahl]

Description

The GetRawElapsedTicks() method dose not handle possebility of negative values.

// Get the elapsed ticks.
private long GetRawElapsedTicks()
{
 long timeElapsed = _elapsed;

 if (_isRunning)
 {
 // If the Stopwatch is running, add elapsed time since
 // the Stopwatch is started last time.
 long currentTimeStamp = GetTimestamp();
 long elapsedUntilNow = currentTimeStamp - _startTimeStamp;
 timeElapsed += elapsedUntilNow;
 }
 return timeElapsed;
}

Simple fix:

private long GetRawElapsedTicks()
{
 long timeElapsed = _elapsed;

 if (_isRunning)
 {
 // If the Stopwatch is running, add elapsed time since
 // the Stopwatch is started last time.
 long currentTimeStamp = GetTimestamp();
 long elapsedUntilNow = currentTimeStamp - _startTimeStamp;

 if (elapsedUntilNow > 0)
 {
 timeElapsed += elapsedUntilNow;
 }
 }
 return timeElapsed;
}

I would like to suggest a little restructuring of the StopMethod on the System.Diagnostics.Stopwatch.

The current implementation allows negativ values in elapsedThisPeriod to be added to _elapsed and resets _elapsed if the total is a negativ value.

public void Stop()
{
 // Calling stop on a stopped Stopwatch is a no-op.
 if (_isRunning)
 {
 long endTimeStamp = GetTimestamp();
 long elapsedThisPeriod = endTimeStamp - _startTimeStamp;
 _elapsed += elapsedThisPeriod;
 _isRunning = false;

 if (_elapsed < 0)
 {
 // 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).

 _elapsed = 0;
 }
 }
}

Would it not make more sens to skip adding negativ values to _elapsed?

 public void Stop()
 {
 // Calling stop on a stopped Stopwatch is a no-op.
 if (_isRunning)
 {
 long endTimeStamp = GetTimestamp();
 long elapsedThisPeriod = endTimeStamp - _startTimeStamp;
 _isRunning = false;

 if (elapsedThisPeriod > 0)
 {
 // 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).

 _elapsed += elapsedThisPeriod;
 }
 }
 }

Or is the rationell due to some performance consideration in regards to the if-statement?

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

Issue Details

---

Description

The GetRawElapsedTicks() method dose not handle possebility of negative values.

// Get the elapsed ticks.
private long GetRawElapsedTicks()
{
 long timeElapsed = _elapsed;

 if (_isRunning)
 {
 // If the Stopwatch is running, add elapsed time since
 // the Stopwatch is started last time.
 long currentTimeStamp = GetTimestamp();
 long elapsedUntilNow = currentTimeStamp - _startTimeStamp;
 timeElapsed += elapsedUntilNow;
 }
 return timeElapsed;
}

Simple fix:

private long GetRawElapsedTicks()
{
 long timeElapsed = _elapsed;

 if (_isRunning)
 {
 // If the Stopwatch is running, add elapsed time since
 // the Stopwatch is started last time.
 long currentTimeStamp = GetTimestamp();
 long elapsedUntilNow = currentTimeStamp - _startTimeStamp;

 if (elapsedUntilNow > 0)
 {
 timeElapsed += elapsedUntilNow;
 }
 }
 return timeElapsed;
}

I would like to suggest a little restructuring of the StopMethod on the System.Diagnostics.Stopwatch.

The current implementation allows negativ values in elapsedThisPeriod to be added to _elapsed and resets _elapsed if the total is a negativ value.

public void Stop()
{
 // Calling stop on a stopped Stopwatch is a no-op.
 if (_isRunning)
 {
 long endTimeStamp = GetTimestamp();
 long elapsedThisPeriod = endTimeStamp - _startTimeStamp;
 _elapsed += elapsedThisPeriod;
 _isRunning = false;

 if (_elapsed < 0)
 {
 // 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).

 _elapsed = 0;
 }
 }
}

Would it not make more sens to skip adding negativ values to _elapsed?

 public void Stop()
 {
 // Calling stop on a stopped Stopwatch is a no-op.
 if (_isRunning)
 {
 long endTimeStamp = GetTimestamp();
 long elapsedThisPeriod = endTimeStamp - _startTimeStamp;
 _isRunning = false;

 if (elapsedThisPeriod > 0)
 {
 // 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).

 _elapsed += elapsedThisPeriod;
 }
 }
 }

Or is the rationell due to some performance consideration in regards to the if-statement?

Author:	andreas-synnerdahl
Assignees:	-
Labels:	area-System.Diagnostics, untriaged
Milestone:	-

[IDisposable]

Does this actually happen, I thought the underlying timestamp sources was monotonically increasing. See here

Is the performance counter monotonic (non-decreasing)?

Yes. QPC does not go backward.

and also won't roll over during a reasonable lifetime

How often does QPC roll over?

Not less than 100 years from the most recent system boot, and potentially longer based on the underlying hardware timer used. For most applications, rollover isn't a concern.

[tannergooding]

Closing as a dupe of #66734.

As described in similar issues/related PRs:

We utilize the monotonic timers available from the underlying OS (QueryPerformanceCounter on Windows, CLOCK_UPTIME_RAW or CLOCK_MONOTONIC on Unix) which have strict contracts around not going backwards,

We can have up to a 1ns resolution, in which case overflow would take ~292 years in the worst possible scenario.

Given the support lifecycle of .NET and pieces of hardware, I'd say you have many other problems if you've not restarted your machine in 292 years ;)