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

dotnet test discovery slow #517

Open
TheAngryByrd opened this issue Feb 15, 2025 · 5 comments
Open

dotnet test discovery slow #517

TheAngryByrd opened this issue Feb 15, 2025 · 5 comments

Comments

@TheAngryByrd
Copy link
Contributor

👋 So I've been annoyed by a specific problem for a while. When running Expecto under dotnet test it seems to take ages to find the tests. This gets really bad for very large test suites. I finally discovered what was happening (yes I know this begins in YoloDev but please bear with me).

YoloDev's test discovery eventually calls getLocation for every test which is in Expecto itself:

let getLocation (asm:Assembly) code =

This is some pretty complicated code trying to find the location of a test. Expecto seems to never actually use this, so it's not affected in dotnet run scenarios.

At this point I don't really have a suggestion, but I wanted to at least bring it up and document it so other people can see the problem and maybe have a discussion about how to do this better.

Related:
ionide/FsAutoComplete#1334

@Numpsy
Copy link
Contributor

Numpsy commented Feb 16, 2025

Looks like the lookup function at

let moduleDefinition = ModuleDefinition.ReadModule(asm.Location, readerParams)

will reread the module definition with Cecil for each test case, which might be duplicating a lot of work if there are many tests?

If that's the case, maybe it'd be possible to cache the data, or maybe have a function to get the source location for multiple test cases at once to reduce the overhead?

@farlee2121
Copy link
Collaborator

farlee2121 commented Feb 16, 2025

I've only superficially interacted with the code location so far.

Reading through it, I think @Numpsy identified a good candidate.

The getMethodName family of code also contains quite a bit of reflection.

Profiling the code would probably be a good next step.

@Numpsy
Copy link
Contributor

Numpsy commented Feb 17, 2025

I had a quick go at doing a profile of

  Impl.testFromThisAssembly()
  |> Option.orDefault (TestList ([], Normal))
  |> Test.toTestCodeList
  |> Seq.iter (fun item->

    Impl.getLocation(Assembly.GetEntryAssembly()) item.test
    |> ignore
    
  )

on the Expecto.Tests test project from this repo and got

Image

Image

@farlee2121
Copy link
Collaborator

That certainly looks like caching module types could make a significant difference.

@TheAngryByrd
Copy link
Contributor Author

  let private moduleDefinitionCache = new ConcurrentDictionary<string, Map<string,TypeDefinition>>()
  let getTypesForAssemblyLocation (asm: Assembly) =

    moduleDefinitionCache.GetOrAdd(asm.Location, valueFactory=(fun loc ->
      let readerParams = ReaderParameters( ReadSymbols = true )
      let moduleDefinition = ModuleDefinition.ReadModule(loc, readerParams)

      seq { for t in moduleDefinition.GetTypes() -> (t.FullName, t) }
      |> Map.ofSeq
    ))

Something like this would probably be good enough

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants