Skip to content

Inline VerifyAccess#4021

Merged
singhashish-wpf merged 3 commits intodotnet:mainfrom
ThomasGoulet73:inline-verifyaccess
Jul 14, 2022
Merged

Inline VerifyAccess#4021
singhashish-wpf merged 3 commits intodotnet:mainfrom
ThomasGoulet73:inline-verifyaccess

Conversation

@ThomasGoulet73
Copy link
Contributor

@ThomasGoulet73 ThomasGoulet73 commented Jan 18, 2021

The method VerifyAccess is not being inlined because of the Exception. Not inlining the throw is enough to inline VerifyAccess because the method CheckAccess already qualifies to be inlined.

I used the following code to benchmark it :
https://gist.github.com/ThomasGoulet73/3dbc0cf5d24ca06b639ba6888c27fe02

Here are the results :

Method count Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated
CheckAccess 10 95.25 ns 1.469 ns 1.374 ns 0.0764 - - 480 B
VerifyAccess 10 143.90 ns 1.693 ns 1.501 ns 0.0763 - - 480 B
CheckAccess_Inlined 10 94.92 ns 1.222 ns 1.020 ns 0.0764 - - 480 B
VerifyAccess_Inlined 10 97.63 ns 0.627 ns 0.556 ns 0.0764 - - 480 B

The perf gain is small but since VerifyAccess is used in a lot of place, any perf gain is good.

In the future, we could create a ThrowHelper like in the runtime repo. To prevent inlining throws which may prevent methods to otherwise be inlined.

@ThomasGoulet73 ThomasGoulet73 requested a review from a team as a code owner January 18, 2021 23:43
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Jan 18, 2021
@ghost ghost requested review from SamBent, fabiant3 and ryalanms January 18, 2021 23:43
@ryalanms ryalanms added Performance Performance related issue Untriaged labels Feb 4, 2021
Base automatically changed from master to main March 17, 2021 17:38
@dipeshmsft dipeshmsft added 📭 waiting-author-feedback To request more information from author. Community Contribution A label for all community Contributions and removed Untriaged labels Jun 23, 2022
@dipeshmsft dipeshmsft added Queue for test pass and removed 📭 waiting-author-feedback To request more information from author. Under Review labels Jun 27, 2022
@dipeshmsft
Copy link
Member

This PR will be taken in the next community test pass.

@singhashish-wpf singhashish-wpf merged commit 93c7f4b into dotnet:main Jul 14, 2022
@ThomasGoulet73 ThomasGoulet73 deleted the inline-verifyaccess branch July 14, 2022 12:59
bgrainger pushed a commit to Faithlife/wpf that referenced this pull request Jul 22, 2022
(cherry picked from commit 93c7f4b)

Conflicts:
	src/Microsoft.DotNet.Wpf/src/WindowsBase/System/Windows/Threading/Dispatcher.cs
@ghost ghost locked as resolved and limited conversation to collaborators Aug 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Community Contribution A label for all community Contributions Performance Performance related issue PR metadata: Label to tag PRs, to facilitate with triage

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

7 participants