Skip to content
New issue

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

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

Already on GitHub? # to your account

Stop instantiating Stopwatch #31348

Merged
merged 2 commits into from
Aug 1, 2023
Merged

Stop instantiating Stopwatch #31348

merged 2 commits into from
Aug 1, 2023

Conversation

roji
Copy link
Member

@roji roji commented Jul 25, 2023

Closes #31347
Closes #26295

/cc @vonzshik

@roji roji requested a review from a team July 25, 2023 10:36
@roji roji changed the title Stop instantiating Stopwatch on net7.0 Stop instantiating Stopwatch Jul 25, 2023
#if NET7_0_OR_GREATER
=> Stopwatch.GetElapsedTime(startTimestamp);
#else
=> new((long)((Stopwatch.GetTimestamp() - startTimestamp) * StopWatchTickFrequency));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copied from the .NET 7.0 implementation

@roji
Copy link
Member Author

roji commented Jul 26, 2023

@AndriySvyryd @bricelam simplified this thanks to @sharwell's suggestion, will wait in case you want to take another look etc.

@roji
Copy link
Member Author

roji commented Aug 1, 2023

OK, will merge this - let me know retroactively if you see any issues.

@roji roji merged commit 2685f70 into dotnet:main Aug 1, 2023
@roji roji deleted the Stopwatch branch August 1, 2023 18:51
@@ -155,12 +154,12 @@ public override bool NextResult()
{
stmt = _stmtEnumerator.Current;

_timer.Start();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 If you're ever interested in adding more of the API, you can reference some old work here:
dotnet/roslyn@ce6b1a9

It wasn't worth the complexity for our uses, so you would make the call. Good to leave a note for future reference though. 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sharwell, yeah - I think the current thing more than fits our needs...

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stop instantiating Stopwatch SqliteCommand allocates one Stopwatch per every query or prepare
4 participants