NotificationsYou must be signed in to change notification settings
Fork5.1k
Star16.6k

Add Capacity property and overloaded TrimExcess#85877

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

Jump to bottom

Merged

eiriktsarpalis merged 17 commits intodotnet:mainfromhrrrrustic:issue-66426

Nov 22, 2023

Merged

Add Capacity property and overloaded TrimExcess#85877

eiriktsarpalis merged 17 commits intodotnet:mainfromhrrrrustic:issue-66426

Nov 22, 2023

Conversation

Copy link

Contributor

hrrrrustic commentedMay 6, 2023

Close#66426

ghost added community-contribution

Indicates that the PR has been added by a community member

area-System.Collections new-api-needs-documentation labels

May 6, 2023

Copy link

ghost commentedMay 6, 2023

Note regarding thenew-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

Copy link

ghost commentedMay 6, 2023

Tagging subscribers to this area: @dotnet/area-system-collections
See info inarea-owners.md if you want to be subscribed.

Issue Details

Close#66426

Author:	hrrrrustic
Assignees:	-
Labels:	`area-System.Collections`,`new-api-needs-documentation`,`community-contribution`
Milestone:	-

Copy link

ContributorAuthor

hrrrrustic commentedMay 7, 2023•
edited
Loading

Hashset &Dictionary ctor tests are failing due to increasing capacity to the nearest prime number. Is it fine to hardcode prime numbers in testdata?

This was referencedMay 7, 2023

Tracking issue for CI build timeouts#76454

Closed

[mono][wasm] System.Numerics.Vectors.Tests assert equal failure#84775

Closed

eiriktsarpalis reviewed

Jun 22, 2023

View reviewed changes

src/libraries/System.Collections/src/System/Collections/Generic/Stack.cs OutdatedShow resolvedHide resolved

eiriktsarpalis reviewed

Jun 22, 2023

View reviewed changes

src/libraries/System.Collections/src/System/Collections/Generic/Stack.csShow resolvedHide resolved

eiriktsarpalis reviewed

Jun 22, 2023

View reviewed changes

src/libraries/System.Collections/tests/Generic/HashSet/HashSet.Generic.Tests.cs Outdated

		AssertExtensions.Throws<ArgumentOutOfRangeException>(() => hashSet.TrimExcess(newCapacity));
		}

		public void HashSet_TrimAccessCurrentCapacity_DoesNothing()

Copy link

Member

eiriktsarpalisJun 22, 2023

Choose a reason for hiding this comment

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

Could you add a test checking that capacity expands after a buffer resize?

eiriktsarpalis reviewed

Jun 22, 2023

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Collections/Generic/HashSet.cs OutdatedShow resolvedHide resolved

eiriktsarpalis reviewed

Jun 22, 2023

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Collections/Generic/HashSet.csShow resolvedHide resolved

eiriktsarpalis reviewed

Jun 22, 2023

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Queue.cs OutdatedShow resolvedHide resolved

eiriktsarpalis requested changes

Jun 22, 2023

View reviewed changes

Copy link

Member

eiriktsarpalis left a comment

Choose a reason for hiding this comment

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

The TrimExcess implementation appears wrong to me for the case wherenewCapacity == Count.

ghost added the needs-author-actionAn issue or pull request that requires more info or actions from the author. label

Jun 22, 2023

eiriktsarpalis reviewed

Jun 22, 2023

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Queue.csShow resolvedHide resolved

ghost added the no-recent-activity label

Jul 6, 2023

Copy link

ghost commentedJul 6, 2023

This pull request has been automatically markedno-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will removeno-recent-activity.

Copy link

ContributorAuthor

hrrrrustic commentedJul 15, 2023•
edited
Loading

Just a comment to remove inactivity label, don't want to open new PR later 😄
I'll fix comments 2-3 weeks later, busy at work for now :(

Btw what's the deadline for shipping in .net8?

ghost removed needs-author-action

An issue or pull request that requires more info or actions from the author.

no-recent-activity labels

Jul 15, 2023

Copy link

Member

danmoseley commentedJul 16, 2023

@jeffhandley can share the cutoff for work like this.
Btw please set PR to Draft if it will be inactive, or just close and reopen later.

hrrrrustic marked this pull request as draft

July 16, 2023 14:20

Copy link

Member

eiriktsarpalis commentedJul 16, 2023

Btw what's the deadline for shipping in .net8?

A bit less than a month left, but do keep in mind that subsequent reviewing rounds can introduce additional delay. If it doesn't make it for .NET 8 that's OK too, we will be accepting .NET 9 contributions as soon as development for .NET 8 concludes.

danmoseley reviewed

Aug 4, 2023

View reviewed changes

src/libraries/System.Collections/src/System/Collections/Generic/Stack.csShow resolvedHide resolved

hrrrrustic marked this pull request as ready for review

August 5, 2023 22:36

hrrrrustic requested a review fromeiriktsarpalis

August 6, 2023 20:08

eiriktsarpalis reviewed

Aug 7, 2023

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Collections/Generic/HashSet.cs OutdatedShow resolvedHide resolved

Copy link

Member

eiriktsarpalis commentedAug 7, 2023•
edited
Loading

We're currently in the process of finalizing .NET 8 work, so we're not accepting any new features. I'll update the milestone and we can revisit this PR in a few weeks when .NET 9 feature work commences.

Copy link

ghost commentedNov 22, 2023

This pull request will now be closed since it had been markedno-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the pull request, but please note that it will be locked if it remains inactive for another 30 days.