haskell / filepath

Haskell FilePath core library
BSD 3-Clause "New" or "Revised" License
66 stars 33 forks source link

Use tasty-bench package itself #181

Closed Bodigrim closed 1 year ago

Bodigrim commented 1 year ago

Resurrecting #108.

The immediate reason is that your fork of tasty-bench is susceptible to https://github.com/Bodigrim/tasty-bench/issues/44, which has been fixed upstream.

There is also an ongoing effort to monitor performance of core libraries between GHC releases, but not using the standard tooling means that filepath is likely to miss out. E. g., there is no comparison between runs available, no filtering by patterns, no easy way to swap tasty-bench for tasty-papi / tasty-perfbench to count CPU instructions.

hasufell commented 1 year ago

So does tasty-bench still depend on optparse-applicative? Can we have a lighter version?

Bodigrim commented 1 year ago

We would not be able to avoid optparse-applicative, but I never had a build issue with it. Any particular reason you deem it undesirable?

hasufell commented 1 year ago

If you rebase against master and then push to this repo instead of your fork, we can benefit from the fixed CI.

Bodigrim commented 1 year ago

cabal bench would take ages on CI. Pass cabal bench --benchmark-options='--timeout 1' (or even cabal bench --benchmark-options='-l' if the goal is just to validate that the executable is runnable).

hasufell commented 1 year ago

It seems to be doing ok, no?

hasufell commented 1 year ago

cabal bench --benchmark-options='--timeout 1'

I guess that makes sense. The benchmark info on CI won't be very useful.

hasufell commented 1 year ago

This is quite interesting:

+ cabal bench
Build profile: -w ghc-9.4.4 -O1
In order, the following will be built (use -v for more details):
 - filepath-1.4.100.1 (bench:bench-filepath) (configuration changed)
Configuring benchmark 'bench-filepath' for filepath-1.4.100.1..
Preprocessing benchmark 'bench-filepath' for filepath-1.4.100.1..
Building benchmark 'bench-filepath' for filepath-1.4.100.1..
Running 1 benchmarks...
Benchmark bench-filepath: RUNNING...

Access violation in generated code when reading 0xe47d8008

 Attempting to reconstruct a stack trace...

   Frame    Code address
 * 0x6d372fcdb0 0x7ff75662577e D:\a\filepath\filepath\dist-newstyle\build\x86_64-windows\ghc-9.4.4\filepath-1.4.100.1\b\bench-filepath\build\bench-filepath\bench-filepath.exe+0xb8577e
 * 0x6d372fce20 0x7ff7566248cc D:\a\filepath\filepath\dist-newstyle\build\x86_64-windows\ghc-9.4.4\filepath-1.4.100.1\b\bench-filepath\build\bench-filepath\bench-filepath.exe+0xb848cc
 * 0x6d372fce70 0x7ff7567a0794 D:\a\filepath\filepath\dist-newstyle\build\x86_64-windows\ghc-9.4.4\filepath-1.4.100.1\b\bench-filepath\build\bench-filepath\bench-filepath.exe+0xd00794
 * 0x6d372fcef0 0x7ff75661c5ab D:\a\filepath\filepath\dist-newstyle\build\x86_64-windows\ghc-9.4.4\filepath-1.4.100.1\b\bench-filepath\build\bench-filepath\bench-filepath.exe+0xb7c5ab
 * 0x6d372fcf40 0x7ff75661c44e D:\a\filepath\filepath\dist-newstyle\build\x86_64-windows\ghc-9.4.4\filepath-1.4.100.1\b\bench-filepath\build\bench-filepath\bench-filepath.exe+0xb7c44e
 * 0x6d372fd1c0 0x7ff7563d72e0 D:\a\filepath\filepath\dist-newstyle\build\x86_64-windows\ghc-9.4.4\filepath-1.4.100.1\b\bench-filepath\build\bench-filepath\bench-filepath.exe+0x9372e0
 * 0x6d372fd250 0x7ff755ee673f D:\a\filepath\filepath\dist-newstyle\build\x86_64-windows\ghc-9.4.4\filepath-1.4.100.1\b\bench-filepath\build\bench-filepath\bench-filepath.exe+0x44673f
 * 0x6d372fd290 0x7ff755ee6913 D:\a\filepath\filepath\dist-newstyle\build\x86_64-windows\ghc-9.4.4\filepath-1.4.100.1\b\bench-filepath\build\bench-filepath\bench-filepath.exe+0x446913
 * 0x6d372fd298 0x7ff755ac559c D:\a\filepath\filepath\dist-newstyle\build\x86_64-windows\ghc-9.4.4\filepath-1.4.100.1\b\bench-filepath\build\bench-filepath\bench-filepath.exe+0x2559c

Benchmark bench-filepath: ERROR
Error: cabal-3.8.1.0.exe: Benchmarks failed for bench:bench-filepath from
filepath-1.4.100.1.

In general, windows on 9.4.4 seems not very reliable, e.g. https://gitlab.haskell.org/ghc/ghc/-/issues/21990

hasufell commented 1 year ago

@Bodigrim ... @bgamari was suggesting to carefully check the FFI calls. I don't see any in filepath wrt benchmarks. Given that the test suite doesn't trigger it, maybe it lies in tasty-bench?

Bodigrim commented 1 year ago

The only FFI in tasty-bench is https://github.com/Bodigrim/tasty-bench/blob/920ca44cd5c45274a8e9ea23b38883bc6278343a/src/Test/Tasty/Bench.hs#L1986-L1987, but it seems benign and tasty-bench builds fine on Windows with GHC 9.4.4 (https://github.com/Bodigrim/tasty-bench/actions/runs/4178046179/jobs/7236375600).

I'll try to reproduce next week, once I get to my Windows machine.

RyanGlScott commented 1 year ago

I am able to reproduce the access violation locally on Windows. Some obversations:

hasufell commented 1 year ago

This is the last version of Win32 that doesn't depend on filepath, which is why cabal picks it. Trying to force a build plan with a later version of Win32 will fail since filepath and Win32 will induce cyclic dependencies.

@Bodigrim this is a problem... tasty-bench depends on tasty, which depends on ansi-terminal, which depends on Win32 explicitly. The inlined version does not have this problem.

Bodigrim commented 1 year ago

This is the last version of Win32 that doesn't depend on filepath, which is why cabal picks it. Trying to force a build plan with a later version of Win32 will fail since filepath and Win32 will induce cyclic dependencies.

@Bodigrim this is a problem... tasty-bench depends on tasty, which depends on ansi-terminal, which depends on Win32 explicitly. The inlined version does not have this problem.

Thanks to @mpilgrem, this is now solved: ansi-terminal-1.0 no longer depends on Win32.

Non-moving GC seems to be solidly broken on Windows though. FWIW since tasty-bench-0.3.2 README stopped suggesting enabling non-moving GC for benchmarks precisely because it's been causing too much trouble.

hasufell commented 1 year ago

Non-moving GC seems to be solidly broken on Windows though. FWIW since tasty-bench-0.3.2 README stopped suggesting enabling non-moving GC for benchmarks precisely because it's been causing too much trouble.

I'm fine with disabling it, either on windows or in general.

hasufell commented 1 year ago

CI passes. Is this still considered a Draft?

Bodigrim commented 1 year ago

@hasufell ready for review now.