Skip to content

Avoid captures in SqlConnection.FlushSpecificWritesAsync#36129

Merged
sharwell merged 2 commits intodotnet:masterfrom
sharwell:writes-no-capture
Jun 10, 2019
Merged

Avoid captures in SqlConnection.FlushSpecificWritesAsync#36129
sharwell merged 2 commits intodotnet:masterfrom
sharwell:writes-no-capture

Conversation

@sharwell
Copy link
Copy Markdown
Contributor

@sharwell sharwell commented Jun 3, 2019 •
edited
Loading

This change produced a 484MB (6.8%) allocation reduction in the scenario described by #36114.

@sharwell
Avoid captures in SqlConnection.FlushSpecificWritesAsync
91f9de9 
This change produced a 484MB (6.8%) allocation reduction in the scenario
described by dotnet#36114.
@sharwell sharwell force-pushed the writes-no-capture branch from 2395ea4 to 799744c Compare June 3, 2019 17:02
@sharwell
Copy link
Copy Markdown
Contributor Author

sharwell commented Jun 3, 2019 •
edited
Loading

does the outer attribute also apply to inner functions?

It does not.

consider doc'ing this as well. anything highly perf sensitive i think deserves as much clarity to prevent regressions as possible :)

Moved the static local function to a instance method so the attribute can be applied.

@CyrusNajmabadi
Copy link
Copy Markdown
Contributor

CyrusNajmabadi commented Jun 3, 2019

Moved the static local function to a static method so the attribute can be applied.

WFM.

@sharwell sharwell force-pushed the writes-no-capture branch from 799744c to dcfa05c Compare June 3, 2019 17:29
@sharwell sharwell mentioned this pull request Jun 3, 2019
Merged
@sharwell
Move local function to member function
3a6e02c 
This change allows the PerformanceSensitiveAttribute to be applied to
the method. See dotnet/csharplang#1888.
@sharwell sharwell force-pushed the writes-no-capture branch from dcfa05c to 3a6e02c Compare June 6, 2019 00:08
@sharwell sharwell marked this pull request as ready for review June 6, 2019 00:08
@sharwell sharwell requested a review from a team as a code owner June 6, 2019 00:08
genlu
genlu approved these changes Jun 10, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@genlu genlu left a comment

Choose a reason for hiding this comment

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

:shipit:

@jinujoseph jinujoseph added this to the 16.2 milestone Jun 10, 2019
@sharwell sharwell merged commit b76fac9 into dotnet:master Jun 10, 2019
@sharwell sharwell deleted the writes-no-capture branch June 10, 2019 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@genlu genlu genlu approved these changes

+2 more reviewers

@CyrusNajmabadi CyrusNajmabadi CyrusNajmabadi left review comments

@Therzok Therzok Therzok approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area-IDE

Projects

None yet

Milestone

16.2

Development

Successfully merging this pull request may close these issues.

5 participants

@sharwell @CyrusNajmabadi @Therzok @genlu @jinujoseph