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

More strongly-type multiple fields and return types #2242

Merged
merged 1 commit into from
Aug 22, 2023

Conversation

stephentoub
Copy link
Collaborator

Some of these, like JsonClaimSet._jsonClaims, help to avoid allocation by ensuring that when the collection is enumerated, we're using its struct enumerator. In other cases, like with CreateClaimFromObject, we reduce the overhead of calls to the object. And other cases are just good hygiene.

Some of these, like JsonClaimSet._jsonClaims, help to avoid allocation by ensuring that when the collection is enumerated, we're using its struct enumerator. In other cases, like with CreateClaimFromObject, we reduce the overhead of calls to the object.  And other cases are just good hygiene.
Copy link
Collaborator

@jennyf19 jennyf19 left a comment

Choose a reason for hiding this comment

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

:shipit: Thanks @stephentoub

@brentschmaltz brentschmaltz added this pull request to the merge queue Aug 22, 2023
Merged via the queue into AzureAD:dev7 with commit e6f0add Aug 22, 2023
@stephentoub stephentoub deleted the strongtype branch August 23, 2023 00:19
@@ -43,16 +43,16 @@ internal IList<Claim> Claims(string issuer)
return _claims;
}

internal IList<Claim> CreateClaims(string issuer)

Choose a reason for hiding this comment

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

@stephentoub Notice you changed all Interfaces to Concrete Subtypes. Just curious what is the benefit or advantage of this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Some of the advantages were noted in the PR description.

For example, if you have:

IList<T> list = ...;
foreach (T item in list) { ... }

that allocates an IEnumerator<T> object. If you instead have:

List<T> list = ...;
foreach (T item in list) { ... }

it doesn't, instead using the List<T>.Enumerator struct for enumeration.

But even just simple accesses, e.g.

IList<T> list = ...;
return list[2];

That involves interface dispatch, which is not only slower than a regular method invocation:

List<T> list = ...;
return list[2];

but without a technology like dynamic PGO, it can't be inlined.

@jennyf19
Copy link
Collaborator

Included in preview4 release

brentschmaltz pushed a commit that referenced this pull request Sep 6, 2023
Some of these, like JsonClaimSet._jsonClaims, help to avoid allocation by ensuring that when the collection is enumerated, we're using its struct enumerator. In other cases, like with CreateClaimFromObject, we reduce the overhead of calls to the object.  And other cases are just good hygiene.
brentschmaltz pushed a commit that referenced this pull request Sep 7, 2023
Some of these, like JsonClaimSet._jsonClaims, help to avoid allocation by ensuring that when the collection is enumerated, we're using its struct enumerator. In other cases, like with CreateClaimFromObject, we reduce the overhead of calls to the object.  And other cases are just good hygiene.
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