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.

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

Perf - Lots of time being spent in HashCompare.GenericCompare due to LowerComputedListOrArrayExpr #13033

Closed
ChrisCanCompute opened this issue Apr 21, 2022 · 4 comments
Labels
Area-Compiler-Checking Type checking, attributes and all aspects of logic checking Feature Improvement Theme-Performance

Comments

@ChrisCanCompute
Copy link

ChrisCanCompute commented Apr 21, 2022

Please provide a succinct description of the issue.

Repro steps

The project I'm seeing this in is the slowest of about 300 in our solution to compile. The project is contains sensitive information, so I'm unable to share it and I have been unable to reproduce this issue in a way that I can share here.
What I can provide is the subtree from a profile of the compiler as it compiles the project.

GenericCompare  •  1,628 ms  •  Microsoft.FSharp.Core.LanguagePrimitives+HashCompare.GenericCompare(GenericComparer, Object, Object)
  92.86%   CompareTo  •  1,512 ms of 1,628  •  System.Tuple`2.CompareTo(Object, IComparer)
    84.02%   tryGetValue  •  1,368 ms of 1,628  •  Microsoft.FSharp.Collections.MapTreeModule.tryGetValue(IComparer, TKey, ref TValue, MapTree)
      71.85%   TryForward  •  1,170 ms of 1,628  •  FSharp.Compiler.TypedTree+CcuThunk.TryForward(String[], String)
        71.85%   tryForwardPrefixPath@3238  •  FSharp.Compiler.TypedTree.tryForwardPrefixPath@3238(String[], CcuThunk, Int32)
          71.85%   TryDeref  •  FSharp.Compiler.TypedTree+NonLocalEntityRef.TryDeref(Boolean)
            71.85%   get_TryDeref  •  FSharp.Compiler.TypedTree+EntityRef.get_TryDeref
              42.74%   LowerComputedListOrArrayExpr  •  696 ms of 1,628  •  FSharp.Compiler.LowerCallsAndSeqs.LowerComputedListOrArrayExpr(FSharpFunc, TcGlobals, ImportMap, Expr)
              29.12%   get_TryDeref  •  474 ms of 1,628  •  FSharp.Compiler.TypedTree+ValRef.get_TryDeref
      12.16%   TryGetValue  •  198 ms of 1,628  •  Microsoft.FSharp.Collections.FSharpMap`2.TryGetValue(TKey, out TValue)
    8.84%   CompareTo  •  144 ms of 1,628  •  System.Tuple`2.CompareTo(Object, IComparer)
  6.16%   tryGetValue  •  100 ms of 1,628  •  Microsoft.FSharp.Collections.MapTreeModule.tryGetValue(IComparer, TKey, ref TValue, MapTree)
  0.98%   add  •  16 ms of 1,628  •  Microsoft.FSharp.Collections.MapTreeModule.add(IComparer, TKey, TValue, MapTree)

For reference, total profiled compile of the project took 29,938 ms, so we're spending ~5% of compile time doing these comparisons.

Expected behavior

Compiler should be quick to run

Actual behavior

Compiler is spending a lot of time in GenericCompare

Known workarounds

N/A

Related information

I'm running with

Microsoft (R) F# Compiler version 12.0.2.0 for F# 6.0

From past experience, swapping a Map for a Dictionary can improve performance in lookups, which would be a change to https://github.com/dotnet/fsharp/blob/main/src/fsharp/TypedTree.fs#L5383
I've not managed to build the compiler inside our company firewall, so I've not been able to try this out.

@ChrisCanCompute ChrisCanCompute changed the title Perf - Lots of time being spent in HashCompare due to LowerComputedListOrArrayExpr Perf - Lots of time being spent in HashCompare.GenericCompare due to LowerComputedListOrArrayExpr Apr 21, 2022
@dsyme
Copy link
Contributor

dsyme commented Apr 25, 2022

The relevant code is a Map lookup on a string[] * string for the CcuTypeForwarderTable

Certainly this can be changed to either a Dictionary using a HashIdentity.Structural comparer, or to a bespoke data structure wrapping a Dictionary using a bespoke implementation of IEqualityComparer

@dsyme dsyme added Feature Improvement Area-Compiler-Checking Type checking, attributes and all aspects of logic checking labels Apr 25, 2022
@ChrisCanCompute
Copy link
Author

I've managed to attach a debugger and see what's in the Map.
I've got ~2500 entries and they're either System.* or Microsoft.Win32.SafeHandles.*. I'm not sure if this is expected or not.

@nojaf
Copy link
Contributor

nojaf commented May 18, 2022

Hello @ChrisCanCompute, would it be ok to close this issue?

@ChrisCanCompute
Copy link
Author

Yep. Thanks Florian!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compiler-Checking Type checking, attributes and all aspects of logic checking Feature Improvement Theme-Performance
Projects
None yet
Development

No branches or pull requests

4 participants