Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

CollectionsMarshal.GetValueRef(Dictionary)#49388

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@benaadams
Copy link
Member

@benaadamsbenaadams commentedMar 9, 2021
edited
Loading

Implement the api

namespaceSystem.Runtime.InteropServices{publicstaticclassCollectionsMarshal{publicrefTValueGetValueRefOrNullRef<TKey,TValue>(Dictionary<TKey,TValue>dictionary,TKeykey);}}

Contributes to#27062

/cc@layomia

omariom, lindexi, and voronov-maxim reacted with thumbs up emoji
@ghost
Copy link

Tagging subscribers to this area:@eiriktsarpalis
See info in area-owners.md if you want to be subscribed.

Issue Details

Still needs doc comments and tests

Resolves#27062

Author:benaadams
Assignees:-
Labels:

area-System.Collections

Milestone:-

@stephentoub
Copy link
Member

Still needs doc comments and tests

Are there places outside of Dictionary we should be using the new method, too?

@benaadams
Copy link
MemberAuthor

Are there places outside of Dictionary we should be using the new method, too?

ForGetValueRef the big wins are for structTValues; either using in place or mutating which don't look to be a common pattern in CoreLib.

HoweverGetValueRefOrAddDefault does look like it would have more applicability removing a double lookup fromTryGet+Add; assuming it was implemented more efficiently. So is probably worth looping back and doing that as well.

@jkotas
Copy link
Member

GetValueRefOrAddDefault does look like it would have more applicability removing a double lookup from TryGet+Add

It also needs to be something reasonably hot to make it worth the trouble.

@benaadams
Copy link
MemberAuthor

It also needs to be something reasonably hot to make it worth the trouble.

While CoreLib does create dictionaries, it doesn't really do much modifying of them and alas my search-fu isn't good enough to check all the other libs; though I definitely have uses further downstream

Looks like a ref enumerator

refstructRefEnumerator{(TKeykey,refTValue)Current{get;}boolMoveNext();}

would be helpful too, but small steps 😉

omariom and lindexi reacted with thumbs up emoji

@benaadamsbenaadamsforce-pushed theCollectionsMarshal-Dictionary branch from310ad9b to66feb3cCompareMarch 10, 2021 03:15
@benaadamsbenaadams reopened thisMar 10, 2021
@benaadamsbenaadamsforce-pushed theCollectionsMarshal-Dictionary branch 3 times, most recently fromb45aaa5 to8d37543CompareMarch 10, 2021 13:24
@benaadamsbenaadams marked this pull request as ready for reviewMarch 10, 2021 13:42
@stephentoub
Copy link
Member

Thanks,@benaadams.

@benaadams
Copy link
MemberAuthor

runtime (Libraries Test Run checked coreclr windows x64 Debug) Failure is#48236 though that's closed, and looks like the vm pool died for the rest

@benaadamsbenaadamsforce-pushed theCollectionsMarshal-Dictionary branch from76c7749 to4b9cc40CompareMarch 17, 2021 02:26
@benaadams
Copy link
MemberAuthor

Rebased to reset CI

@benaadams
Copy link
MemberAuthor

🥳

Joe4evr reacted with hooray emoji

@stephentoubstephentoub merged commit46127ea intodotnet:mainMar 17, 2021
@benaadamsbenaadams deleted the CollectionsMarshal-Dictionary branchMarch 17, 2021 10:50
@ghostghost locked asresolvedand limited conversation to collaboratorsApr 16, 2021
@karelzkarelz added this to the6.0.0 milestoneMay 20, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@jkotasjkotasjkotas left review comments

@tannergoodingtannergoodingtannergooding left review comments

@stephentoubstephentoubstephentoub approved these changes

@eiriktsarpaliseiriktsarpaliseiriktsarpalis approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

6.0.0

Development

Successfully merging this pull request may close these issues.

6 participants

@benaadams@stephentoub@jkotas@eiriktsarpalis@tannergooding@karelz

[8]ページ先頭

©2009-2025 Movatter.jp