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.

Sign up for GitHub

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

Jump to bottom

Dataflow analysis lockup compiling System.Runtime.Serialization.Formatters.Tests #73048

Closed
MichalStrehovsky opened this issue Jul 29, 2022 · 3 comments · Fixed by #74111
Closed

Dataflow analysis lockup compiling System.Runtime.Serialization.Formatters.Tests #73048

MichalStrehovsky opened this issue Jul 29, 2022 · 3 comments · Fixed by #74111
Labels
area-NativeAOT-coreclr
Milestone
7.0.0

Comments

@MichalStrehovsky
Copy link
Member

It's possible this is an illinker bug, but I'm not ready to spend time extracting an IL Linker repro:

To repro, build the repo how you build it (e.g. build.cmd clr.aot+libs -rc Debug -lc Release). Then:

pushd src\libraries\System.Runtime.Serialization.Formatters\tests
dotnet build -c Release /p:TestNativeAot=true /p:UseNativeAotCoreLib=true /p:RuntimeConfiguration=Debug /p:LibrariesConfiguration=Release /p:TargetFramework=net7.0-windows

Once it tells you it's generating native code, taskkill ilc.exe and find the response file to invoke ilc.exe under artifacts\obj\System.Runtime.Serialization.Formatters.Tests\Release\net7.0-windows\native. Open src\coreclr\tools\aot\ilc.sln in VS, set configuration to the one you built (Debug in my case from the -rc Debug argument), and set the response file as the command line argument. You might want to append --parallelism:1 to make things not multithreaded.

The method we're stuck on is [System.Runtime.Serialization.Formatters.Tests]System.Runtime.Serialization.Formatters.Tests.BinaryFormatterTests.SerializableObjects()

Example callstack:

 	System.Private.CoreLib.dll!System.RuntimeType.GetFieldCandidates(string name, System.Reflection.BindingFlags bindingAttr, bool allowPrefixLookup)	Unknown
 	System.Private.CoreLib.dll!System.RuntimeType.GetFields(System.Reflection.BindingFlags bindingAttr)	Unknown
 	System.Private.CoreLib.dll!System.ValueType.Equals(object obj)	Unknown
 	System.Private.CoreLib.dll!System.Collections.Generic.HashSet<System.Collections.Generic.KeyValuePair<int, ILCompiler.Dataflow.ValueBasicBlockPair>>.Remove(System.Collections.Generic.KeyValuePair<int, ILCompiler.Dataflow.ValueBasicBlockPair> item)	Unknown
 	System.Private.CoreLib.dll!System.Collections.Generic.HashSet<System.Collections.Generic.KeyValuePair<int, ILCompiler.Dataflow.ValueBasicBlockPair>>.ExceptWith(System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<int, ILCompiler.Dataflow.ValueBasicBlockPair>> other)	Unknown
>	ILCompiler.Compiler.dll!ILLink.Shared.TrimAnalysis.ArrayValue.Equals(ILLink.Shared.TrimAnalysis.ArrayValue otherArr) Line 79	C#
 	System.Private.CoreLib.dll!System.Collections.Generic.GenericEqualityComparer<ILLink.Shared.DataFlow.SingleValue>.Equals(ILLink.Shared.DataFlow.SingleValue x, ILLink.Shared.DataFlow.SingleValue y)	Unknown
 	System.Private.CoreLib.dll!System.Collections.Generic.HashSet<ILLink.Shared.DataFlow.SingleValue>.AddIfNotPresent(ILLink.Shared.DataFlow.SingleValue value, out int location)	Unknown
 	System.Private.CoreLib.dll!System.Collections.Generic.HashSet<ILLink.Shared.DataFlow.SingleValue>.UnionWith(System.Collections.Generic.IEnumerable<ILLink.Shared.DataFlow.SingleValue> other)	Unknown
 	System.Private.CoreLib.dll!System.Collections.Generic.HashSet<ILLink.Shared.DataFlow.SingleValue>.HashSet(System.Collections.Generic.IEnumerable<ILLink.Shared.DataFlow.SingleValue> collection, System.Collections.Generic.IEqualityComparer<ILLink.Shared.DataFlow.SingleValue> comparer)	Unknown
 	ILCompiler.Compiler.dll!ILLink.Shared.DataFlow.ValueSet<ILLink.Shared.DataFlow.SingleValue>.Clone() Line 190	C#
 	ILCompiler.Compiler.dll!ILLink.Shared.DataFlow.ValueSet<ILLink.Shared.DataFlow.SingleValue>.Meet(ILLink.Shared.DataFlow.ValueSet<ILLink.Shared.DataFlow.SingleValue> left, ILLink.Shared.DataFlow.ValueSet<ILLink.Shared.DataFlow.SingleValue> right) Line 159	C#
 	ILCompiler.Compiler.dll!ILCompiler.Dataflow.MethodBodyScanner.ScanStfld(Internal.IL.MethodIL methodBody, int offset, Internal.IL.ILOpcode opcode, Internal.TypeSystem.FieldDesc field, System.Collections.Generic.Stack<ILCompiler.Dataflow.StackSlot> currentStack, ref ILCompiler.Dataflow.InterproceduralState interproceduralState) Line 1134	C#
 	ILCompiler.Compiler.dll!ILCompiler.Dataflow.MethodBodyScanner.Scan(Internal.IL.MethodIL methodBody, ref ILCompiler.Dataflow.InterproceduralState interproceduralState) Line 664	C#
 	ILCompiler.Compiler.dll!ILCompiler.Dataflow.ReflectionMethodBodyScanner.Scan(Internal.IL.MethodIL methodBody, ref ILCompiler.Dataflow.InterproceduralState interproceduralState) Line 106	C#
 	ILCompiler.Compiler.dll!ILCompiler.Dataflow.MethodBodyScanner.InterproceduralScan(Internal.IL.MethodIL startingMethodBody) Line 317	C#
 	ILCompiler.Compiler.dll!ILCompiler.Dataflow.ReflectionMethodBodyScanner.InterproceduralScan(Internal.IL.MethodIL methodBody) Line 97	C#
 	ILCompiler.Compiler.dll!ILCompiler.Dataflow.ReflectionMethodBodyScanner.ScanAndProcessReturnValue(ILCompiler.DependencyAnalysis.NodeFactory factory, ILLink.Shared.TrimAnalysis.FlowAnnotations annotations, ILCompiler.Logger logger, Internal.IL.MethodIL methodBody) Line 121	C#
 	ILCompiler.Compiler.dll!ILCompiler.DependencyAnalysis.DataflowAnalyzedMethodNode.GetStaticDependencies(ILCompiler.DependencyAnalysis.NodeFactory factory) Line 36	C#

Cc @vitek-karas @sbomer
@MichalStrehovsky MichalStrehovsky added this to the 7.0.0 milestone Jul 29, 2022
@MichalStrehovsky MichalStrehovsky mentioned this issue Jul 29, 2022
Open
@vitek-karas
Copy link
Member

I must be doing something wrong - the above repro steps don't hang. ILC successfully process the input in about 20 seconds (maybe a tiny bit more). This is on very recent main branch - no local changes.

@MichalStrehovsky
Copy link
Member Author

Tried enabling the test in #73104.

The CI has been locked up like this for 20 minutes.

  System.Runtime.Serialization.Formatters.Tests -> /__w/1/s/artifacts/bin/System.Runtime.Serialization.Formatters.Tests/Release/net7.0-linux/System.Runtime.Serialization.Formatters.Tests.dll
  Optimizing assemblies for size may change the behavior of the app. Be sure to test after publishing. See: https://aka.ms/dotnet-illink
  Generating compatible native code. To optimize for size or speed, visit https://aka.ms/OptimizeNativeAOT

It's possible the repro steps don't repro it. I'm still trying to figure out what is the right way to dotnet build a single test. What I wrote above has worked for me in the past but maybe it requires some previous artifacts leftover from a previous build attempt. Not sure.

What I always do locally is I change the definition of SmokeTestProject in src/libraries/tests.proj to only have the test of interest to me and run build.cmd libs.tests -rc Debug -lc Release /p:TestNativeAot=true /p:RunSmokeTestsOnly=true /p:UseNativeAotCoreLib=true. It's not nice but it works :/

@MichalStrehovsky MichalStrehovsky mentioned this issue Aug 8, 2022
Closed
MichalStrehovsky added a commit to MichalStrehovsky/linker that referenced this issue Aug 17, 2022
@MichalStrehovsky
Add test for dotnet/runtime#73048
42ab36f 
Running dataflow analysis on the added test takes about 20 seconds. Double the number of `yield returns` and it will take several minutes.

In the runtime repo we're running into this on https://github.com/dotnet/runtime/blob/main/src/libraries/System.Runtime.Serialization.Formatters/tests/BinaryFormatterTestData.cs (method `SerializableObjects()`). That one has even more `yield returns` and I lost patience waiting for it to finish.
@MichalStrehovsky
Copy link
Member Author

It's probably not a lockup - I just don't have enough patience. Extracted a repro to dotnet/linker#2977.

sbomer pushed a commit to dotnet/linker that referenced this issue Aug 17, 2022
@MichalStrehovsky
Add test for dotnet/runtime#73048 (#2977)
c42e3cb 
Running dataflow analysis on the added test takes about 20 seconds. Double the number of `yield returns` and it will take several minutes.

In the runtime repo we're running into this on https://github.com/dotnet/runtime/blob/main/src/libraries/System.Runtime.Serialization.Formatters/tests/BinaryFormatterTestData.cs (method `SerializableObjects()`). That one has even more `yield returns` and I lost patience waiting for it to finish.
MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Aug 17, 2022
@MichalStrehovsky
Don't track current field of state machines
21423d5 
Port of dotnet/linker#2979

Fixes dotnet#73048.
@MichalStrehovsky MichalStrehovsky mentioned this issue Aug 17, 2022
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 17, 2022
MichalStrehovsky added a commit that referenced this issue Aug 19, 2022
@MichalStrehovsky
Don't track current field of state machines (#74111)
3932882 
Port of dotnet/linker#2979

Fixes #73048.
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 19, 2022
github-actions bot pushed a commit that referenced this issue Aug 19, 2022
@MichalStrehovsky
Don't track current field of state machines
ace4cd3 
Port of dotnet/linker#2979

Fixes #73048.
jeffschwMSFT pushed a commit that referenced this issue Aug 20, 2022
@github-actions @MichalStrehovsky
Don't track current field of state machines (#74216)
37193fd 
Port of dotnet/linker#2979

Fixes #73048.

Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
@ghost ghost locked as resolved and limited conversation to collaborators Sep 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area-NativeAOT-coreclr
Projects
None yet
Milestone
7.0.0
Development

Successfully merging a pull request may close this issue.

Don't track current field of state machines MichalStrehovsky/runtime
2 participants
@vitek-karas @MichalStrehovsky