Add ConcurrentBag<T> Clear method

[AlexRadch]

Implement ConcurrentBag.Clear API. See https://github.com/dotnet/corefx/issues/2338

[karelz]

Please add tests: https://github.com/dotnet/corefx/blob/master/src/System.Collections.Concurrent/tests/ConcurrentBagTests.cs

[stephentoub]

Side note, @AlexRadch, before you submit PRs, can you please make sure your branch is up-to-date with master, so as to avoid all of the extra commits like "Merge pull request #2 from dotnet/master"?

[stephentoub]

This is not a safe change. It would need to synchronize with all of the lists, which means freezing the bag before making these changes. And you shouldn't need to allocate a new thread local; you should be able to freeze the bag, iterate through all of the local lists clearing each, then unfreeze the bag.

[AlexRadch]

You don't right.
Method FreezeBag used by read only methods such like: CopyTo, ToArray, GetEnumerator, Count and IsEmpty.
Methods that change bag use lock (GlobalListsLock) see GetThreadList method that used by next methods: Initialize, Add, TryTakeOrPeek.
So my lock is correct.
ThreadLocal class do not have methods to clear. So we must recreate it. After that all local lists will be garbage by GC because we do have any reference to them.

[stephentoub]

No. By doing it the way you're doing it, Clear can end up doing only a partial clear, where it can still be clearing from part of the data structure while new elements are being added elsewhere that won't be cleared; you can also end up in situations where you'll end up clearing elements added after the Clear operation completes. This is why you need to take all the locks, not just the global one (which freezing also takes). Plus, doing it that way means you don't have to throw away the thread local and all the non-trivial data structures it references.

[AlexRadch]

@stephentoub I don't understand how to sync my corefx repository to dotnet/corefx without such sync PRs. I clone corefx some time ago and now to sync my copy I read instruction on Stack Exchange but it creates many sync PRs that you see.
Can you give me right instruction, please?

[stephentoub]

I don't understand how to sync my corefx repository to dotnet/corefx without such sync PRs

Do you have a remote for your local that points to the dotnet corefx repo on GitHub? If not, create one with a command like:

git remote add upstream https://github.com/dotnet/corefx.git

Just like "origin" points to your GitHub repo (by default), with that command "upstream" will point to the dotnet one. Then you can do a command like:

git checkout master
git pull upstream master

That'll switch to your local master branch and then pull down the remote master, making your master up-to-date with the remote one. After that you can also update your remote origin master, e.g.

git push origin master

You can also just rebase your commit on top of the remote upstream master, e.g.

git fetch --all
git checkout ConcurrentBag.Clear
git rebase upstream/master

That makes sure you have a local copy of everything from both origin and upstream, then switches to your local branch for this PR, and then rebases it on top of the master from upstream (takes your new commits and replays them on top of what's in upstream/master).

[AlexRadch]

You don't right.
Method FreezeBag used by read only methods such like: CopyTo, ToArray, GetEnumerator, Count and IsEmpty.
Methods that change bag use lock (GlobalListsLock) see GetThreadList method that used by next methods: Initialize, Add, TryTakeOrPeek.
So my lock is correct.
ThreadLocal class do not have methods to clear. So we must recreate it. After that all local lists will be garbage by GC because we do have any reference to them.

[AlexRadch]

Now I will add tests.

[AlexRadch]

I removed strange Commits.

[AlexRadch]

I created RTest11_Clear() test for new Clear method, but can not compile it with next error

ConcurrentBagTests.cs(492,33): error CS1061: 'ConcurrentBag<int>' does not cont
ain a definition for 'Clear' and no extension method 'Clear' accepting a first
argument of type 'ConcurrentBag<int>' could be found (are you missing a using d
irective or an assembly reference?) [C:\mount\corefx\src\System.Collections.Con
current\tests\System.Collections.Concurrent.Tests.csproj]
Done Building Project "C:\mount\corefx\src\System.Collections.Concurrent\tests\
System.Collections.Concurrent.Tests.csproj" (build target(s)) -- FAILED.

It seem that test project do not see code changes in Bag class. How I can compile tests with new Bag class?

[karelz]

You need to expose the API in netcoreapp11 ref and then create test project specific for netcoreapp11 -- see for example 67bc5d4 or any PR which added a new .NET Core-only API recently (list of issues).

It is not well documented in our how-to-contribute guide yet (I plan to get to it in December - gotta try it myself first :)).

[AlexRadch]

I added netcoreapp11 ref and moved Bag.Clear test to netcoreapp11 tests. Now test project is compiled. But my tests are not even compiled.
How can I compile and run netcoreapp11 tests for project System.Collections.Concurrent.Tests.csproj ?

[AlexRadch]

I am trying add new API to ConcurentBag. I am reading Add APIs.

Now System.Collections.Concurrent.Tests.csproj can be compiled and run local but gloabalbuild return strange build errors. Can any help to me to add API correct?

Determining versions and targets

Determine what library

System.Collections.Concurrent

Determine target framework

• If the library is part of netstandard
  TRUE - It is part of https://github.com/dotnet/corefx/blob/master/pkg/NETStandard.Library/NETStandard.Library.packages.targets
  • Your target framework should be netstandard<latest>
  • If it is a new API only available on .NET Core then it will be added to netcoreapp<latest>
    I think second is false so netstandard1.7

netstandard1.7 Am I right?

Determine library version

• If targeting netstandard TRUE
  • Ensure minor version of the assembly is bumped since last stable package release

(We should bump)

Making the changes in repo

If changing the library version TRUE

• Update the AssemblyVersion property in \dir.props (ex: System.Runtime\dir.props) to the version determined above.
  AssemblyVersion is 4.0.14.0
  Should I bump to 4.1.14.0 or 4.1.0.0 or something else?
  I bump to 4.1.0.0 Am I right?

Update ref

• If changing target framework TRUE

  • Update the NugetTargetMoniker property in <Library>\ref\<Library>.csproj (ex: ref\System.Runtime.csproj)
    I updated from <NuGetTargetMoniker>.NETStandard,Version=v1.3</NuGetTargetMoniker> to <NuGetTargetMoniker>.NETStandard,Version=v1.7</NuGetTargetMoniker>
  • Update frameworks section in <Library>\ref\project.json to add a new target framework or replace an existing one (ex: System.Runtime\ref\project.json)
    I replaced "netstandard1.3": {} to "netstandard1.7": {} Am I right?

• Add your API to <Library>\ref\<Library>.cs. Use GenAPI to normalize the file.
  I added public void Clear() { throw null; }
  What is GenAPI? How to use GenAPI to normalize the file? I did not find GenAPI and any instruction.

• If adding an additional target framework for the ref (ex: adding a .NET Core specific API) you may need to add a <Library>\ref\<Library>.builds for the new configuration (ex: ref/System.Runtime.Extensions.builds).
  System.Collections.Concurrent\ref\System.Collections.Concurrent.builds don't exists. So I spiked this step.

Update src

• If changing target framework
  • Update the NugetTargetMoniker property in <Library>\src\<Library>.csproj (ex: src\System.Runtime.csproj)
    I updated from <NuGetTargetMoniker>.NETStandard,Version=v1.3</NuGetTargetMoniker> to <NuGetTargetMoniker>.NETStandard,Version=v1.7</NuGetTargetMoniker>
  • Update frameworks section in <Library>\src\project.json to add a new target framework or replace an existing one (ex: System.Runtime\src\project.json)
    I updated nothing
  • Update build configurations in <Library>\src\<Library>.builds if necessary (see Project configuration conventions)
    I updated nothing
• Make your code changes.
  I made

Update pkg
- Update SupportedFramework metadata on the ref ProjectReference to declare the set of concrete of platforms you expect your library to support. (see Specific platform mappings). Generally will be a combination of netcoreapp1.x, netfx46x, uap10.x, and/or $(AllXamarinFrameworks).
I updated nothing
- If package is not split (i.e. only one pkgproj, ex pkg\System.Diagnostics.Process.pkgproj) then it should already have a ProjectReference the <Library>\src\<Library>.builds file and so there is no additional changes needed in the pkgproj.

• If assembly or package version is updated the package index needs to be updated by running
  msbuild <Library>/pkg/<Library>.pkgproj /t:UpdatePackageIndex
  I run msbuild src\System.Collections.Concurrent\pkg\System.Collections.Concurrent.pkgproj /t:UpdatePackageIndex

Update tests

• If changing target framework
  • Update the NugetTargetMoniker property in <Library>\tests\<Library>.csproj (ex: tests\System.Runtime.csproj). Add a '$(TargetGroup)' == '' condition if missing.
    I updated nothing. It is .NETStandard,Version=v1.7
  • Update frameworks section in <Library>\tests\project.json to add a new target framework or replace an existing one (ex: System.Runtime\tests\project.json)
    I updated nothing. Such file does not exists.
  • Add new build configuration in <Library>\tests\<Library>.builds (ex: tests\System.Runtime.builds)
    • Set TargetGroup which will generally match the TargetGroup in the src library build configruation. (ex: tests\System.Runtime.builds)
    • Set TestTFMs metadata to a list of target frameworks to run on, will generally match the SupportedFramework metadata in the pkgproj (ex: tests\System.Runtime.builds)
    • If TestTFMs is empty it defaults to netcoreapp1.0
  • I added

    <Project Include="System.Collections.Concurrent.Tests.csproj">
      <TargetGroup>netstandard1.7</TargetGroup>
      <TestTFMs>netcoreapp1.1</TestTFMs>
    </Project>

• Add new test code following conventions for new files to that are specific to the new target framework.
  I added ConcurrentBagTests.netstandard1.7.cs

[AlexRadch]

I recreated this PRs #14084