tweag / ormolu

A formatter for Haskell source code
https://ormolu-live.tweag.io
Other
936 stars 83 forks source link

Format multiple files in parallel #1128

Open michaelpj opened 2 days ago

michaelpj commented 2 days ago

Is your feature request related to a problem? Please describe. At work we use treefmt, which calls a formatter with all the matching files in the repository, i.e. we end up calling ormolu <list of all our haskell files>.

We have ~2.5k Haskell files, and this takes about 32 seconds.

Describe the solution you'd like

One obvious solution would be to just run the files in parallel. That could be as simple as replacing the mapM here with mapConcurrently.

Describe alternatives you've considered

michaelpj commented 2 days ago

I made https://github.com/numtide/treefmt/issues/333 on treefmt, let's see what they say also.

amesgen commented 2 days ago

Sounds sensible!

There was this attempt in the past: #896

Get treefmt to call ormolu once per file, in parallel. This would also be perfectly sensible.

Just to get an idea of how much this would speed things up in your case, you could try running with fd:

fd -e hs . /path/to/my/dir -x ormolu -m check
michaelpj commented 2 days ago

Great idea. I did that, and it didn't actually help that much! Still takes about 25s. I'm unsure why this is.

amesgen commented 2 days ago

Hmm :thinking: Might be interesting to run a profiled version of Ormolu to see if there are any pathologies (such as the one that prompted #896), but I assume your code isn't open source?

michaelpj commented 2 days ago

Sadly no, but I can probably build a profiled ormolu pretty easily and give it a try myself. Seems like I'm not the first person to hit this :joy:

michaelpj commented 2 days ago

Here's a flamegraph of running a profiled ormolu (well, it's actually fourmolu) on all the files. Sure looks like it's bound by the parser, which I would expect to be parallelizable :thinking:

fourmolu

michaelpj commented 1 day ago

A data point: I used the new treefmt which runs formatters on batches of files, currently batches of 1024. So it formats our code in two batches. That is almost 2x faster, which suggests that there is benefit to be had here.

michaelpj commented 1 day ago

Okay, I got a 3x improvement for just what I suggested (replacing mapM with mapConcurrently). I think maybe https://github.com/tweag/ormolu/pull/896 didn't work because the executable wasn't being built with -threaded :sweat_smile:

I can resurrect that PR or similar if you've got a preference for whether or not this should be configurable. mapConcurrently will use all the concurrency it can get which is probably wrong. But then you don't like configuration :)