Update BuildTools, CoreClr, CoreFx, CoreSetup, ProjectNTfs, ProjectNTfsTestILC to preview1-02815-01, preview1-26515-05, preview1-26515-04, preview1-26515-01, beta-26515-00, beta-26515-00, respectively (master)#29702
Conversation
|
Discarded CI Status: 7:x: 4:hourglass: 3:heavy_check_mark: (click to expand)
|
e25f660 to
e26869c
Compare
|
Discarded CI Status: 9:x: 5:heavy_check_mark: (click to expand)
|
e26869c to
d884e87
Compare
|
Discarded CI Status: 8:x: 2:hourglass: 4:heavy_check_mark: (click to expand)
|
d884e87 to
998f2fe
Compare
|
Discarded CI Status: 11:x: 3:heavy_check_mark: (click to expand)
|
998f2fe to
0dda5bc
Compare
|
@maryamariyan I assume that this needs your ConcurrentQueue changes |
|
Discarded CI Status: 10:x: 4:heavy_check_mark: (click to expand)
|
…fsTestILC to preview1-02815-01, preview1-26515-05, preview1-26515-04, preview1-26515-01, beta-26515-00, beta-26515-00, respectively
0dda5bc to
11f19e9
Compare
|
@jkotas thanks. I'm looking at this. |
| <Compile Include="$(CommonPath)\CoreLib\Internal\Padding.cs"> | ||
| <Link>Common\CoreLib\Internal\Padding.cs</Link> | ||
| </Compile> | ||
| <Compile Include="$(CommonPath)\CoreLib\System\Collections\Concurrent\ConcurrentQueue_Segment.cs"> |
There was a problem hiding this comment.
Is ConcurrentQueue_Segment really needed here?
There was a problem hiding this comment.
It shouldn't be. It should only be needed by ConcurrentQueue, which isn't here any more.
There was a problem hiding this comment.
yes. I removed it. Will push up the PR along with a fix for the tests
|
@jkotas I'm working on fixing the failing tests. Should be a straightforward fix |
| /// </para> | ||
| /// </remarks> | ||
| [DebuggerTypeProxy(typeof(IProducerConsumerCollectionDebugView<>))] | ||
| [DebuggerTypeProxy(typeof(ProducerConsumerCollectionDebugView<>))] |
There was a problem hiding this comment.
Would a better name for this be IProducerConsumerCollectionDebugView? It would match convention that we are using in other places (ICollectionDebugView, IDictionaryDebugView, ...).
There was a problem hiding this comment.
That fix will have to go in coreclr
There was a problem hiding this comment.
It does not need to go in coreclr first. You can make it in corefx and let the mirror to bring it over to CoreCLR.
| <Compile Include="System\Net\Http\SocketsHttpHandler\RedirectHandler.cs" /> | ||
| <Compile Include="$(CommonPath)\System\Collections\Concurrent\ConcurrentQueue_Segment.cs"> | ||
| <Link>Common\System\Collections\Concurrent\ConcurrentQueue_Segment.cs</Link> | ||
| <Compile Include="$(CommonPath)\CoreLib\Internal\Padding.cs"> |
There was a problem hiding this comment.
Padding.cs from CoreLib has ARM64-specific ifdefs, but the corefx libraries are not compiled machine specific. We may want to keep a different Padding.cs for corefx with no ifdefs and conservative size that is not machine specific.
There was a problem hiding this comment.
How do you recommend I approach this?
There was a problem hiding this comment.
Have a different Padding.cs for CoreFX without ifdefs for now.
More discussion about this is in #22724
| ConcurrentQueue<ConnectEventArgs>.Segment.RoundUpToPowerOf2(Math.Max(2, Environment.ProcessorCount))); | ||
| private static readonly Segment<ConnectEventArgs> s_connectEventArgs = | ||
| new Segment<ConnectEventArgs>( | ||
| Segment<ConnectEventArgs>.RoundUpToPowerOf2(Math.Max(2, Environment.ProcessorCount))); |
There was a problem hiding this comment.
I didn't notice this in the coreclr PR, but "Segment" is too generic a name for this. When it was a nested type, it made sense, but now that it's not, we should rename it to something else, either ConcurrentQueueSegment or BoundedConcurrentQueue or something like that.
|
Couldn't update this pull request: Head commit author 'Maryam Ariyan' is not 'dotnet-maestro-bot' |
|
Couldn't update this pull request: Head commit author 'Maryam Ariyan' is not 'dotnet-maestro-bot' |
da427b0 to
49e9114
Compare
| </ItemDefinitionGroup> | ||
| <ItemGroup> | ||
| <Compile Include="$(MSBuildThisFileDirectory)Internal\IO\File.cs" /> | ||
| <Compile Include="$(MSBuildThisFileDirectory)Internal\Padding.cs" /> |
| @@ -6,21 +6,15 @@ | |||
|
|
|||
| namespace Internal | |||
There was a problem hiding this comment.
This should be copy, not rename.
| @@ -4,6 +4,8 @@ | |||
| <BuildConfigurations> | |||
| netcoreapp; | |||
There was a problem hiding this comment.
We should not need netcoreapp build configuration now that we have netcoreapp--Window_NT and netcoreapp-Unix configurations.
| <Compile Include="System\Collections\Concurrent\CDSCollectionETWBCLProvider.cs" /> | ||
| <Compile Include="System\Collections\Concurrent\ConcurrentBag.cs" /> | ||
| <Compile Include="System\Collections\Concurrent\ConcurrentDictionary.cs" /> | ||
| <Compile Include="System\Collections\Concurrent\ConcurrentQueue.cs" /> |
There was a problem hiding this comment.
You will need to keep these .cs files in for uapaot build until the change propagates to ProjectN.
|
Couldn't update this pull request: Head commit author 'Maryam Ariyan' is not 'dotnet-maestro-bot' |
|
Couldn't update this pull request: Head commit author 'Maryam Ariyan' is not 'dotnet-maestro-bot' |
421b89e to
872137e
Compare
|
@dotnet-bot test UWP NETNative x86 Release Build |
872137e to
3a53497
Compare
Fixes: dotnet/coreclr#17751
3a53497 to
f56918a
Compare
|
Couldn't update this pull request: Head commit author 'Maryam Ariyan' is not 'dotnet-maestro-bot' |
|
Couldn't update this pull request: Head commit author 'Maryam Ariyan' is not 'dotnet-maestro-bot' |
|
Couldn't update this pull request: Head commit author 'Maryam Ariyan' is not 'dotnet-maestro-bot' |
|
Closing to pick up CoreCLR update |
No description provided.