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

perf: Reduce tracing related allocations from TVP write #557

Merged
merged 2 commits into from
May 15, 2020

Conversation

Wraith2
Copy link
Contributor

@Wraith2 Wraith2 commented May 8, 2020

There is a case where extended metadata trace information is formatted before being passed into an event tracing method which then may not use the string.
I chose not to try and make the tracing code more efficient because it will almost never be used. I've added precheck to see if advanced logging is enabled and passed that as a parameter to the function logging the information because it avoids a per-loop invocation of the event trace flag check.

Method Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
SyncWriteTableParam master 191.0 ms 1.69 ms 1.41 ms 1000.0000 - - 12.05 MB
SyncWriteTableParam branch 185.2 ms 0.42 ms 0.33 ms - - - 313.33 KB

@cheenamalhotra cheenamalhotra added the NetFx 👈 NetCore Issues that require porting from .NET Core to .NET Framework label May 8, 2020
cheenamalhotra added a commit to cheenamalhotra/SqlClient that referenced this pull request May 15, 2020
@cheenamalhotra cheenamalhotra added this to the 2.0.0-preview4 milestone May 15, 2020
@cheenamalhotra cheenamalhotra removed the NetFx 👈 NetCore Issues that require porting from .NET Core to .NET Framework label May 15, 2020
@cheenamalhotra cheenamalhotra merged commit 3e5e84c into dotnet:master May 15, 2020
cheenamalhotra added a commit that referenced this pull request May 15, 2020
@Wraith2 Wraith2 deleted the perf-optionalwritetrace branch May 15, 2020 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants