Closed lydell closed 1 year ago
Awesome, thanks!!
Apologies, I'll probably be a little bit slow on merging this since I want to re-read the InfoFormatter changes a couple times, and make sure I personally understand how Haskell's laziness works w/r to the new code. (I trust that doesn't really break anything, both from a quick skim of the code, and from the testing you've done; but I want to put a bit higher of a bar on myself to understand what the runtime behavior actually is.)
Here are a couple things I'll want to take a look at before I merge. Feel free to take a stab at them if you want, or no worries if you don't, I should have a chance to get to them in the next couple weeks.
World
instance, which would be something like type RealWorld = ReaderT (MVar ()) IO
, make a MonadIO RealWorld
instance (maybe? I think this might make it less verbose to refactor the World instance? -- edit: oh, maybe ReaderT r IO
already has a MonadIO
instance?), and then refactor instance World IO
to be instance World RealWorld
(and the top-level entrypoint then will create the printLock and use runReaderT printLock
)Testing version for MacOS arm64 is available here: https://github.com/avh4/elm-format/actions/runs/4141690402
Just an additional benchmark on a Mac M1 using --validate
on ~150k LOC:
(using https://github.com/sharkdp/hyperfine hyperfine --warmup 3 --shell=none
)
Version | Mean [s] | Min [s] | Max [s] | Relative |
---|---|---|---|---|
darwin-arm64 | ||||
this PR | 0.824 ± 0.006 | 0.815 | 0.832 | 1.00 |
main |
2.776 ± 0.015 | 2.758 | 2.805 | 3.37 ± 0.03 |
darwin-x86 via Rosetta | ||||
this PR | 1.058 ± 0.012 | 1.042 | 1.085 | 1.28 ± 0.02 |
main |
9.603 ± 0.096 | 9.475 | 9.770 | 11.65 ± 0.14 |
I'm thinking using the print lock, it should be possible now to have the forked jobs just print their output, and then only have to pass back a Bool to the parent, and that maybe also would restore the streaming output of the json
I took a brief look at doing this now, but it doesn't seem worth the time atm. Doing it in a clean way seems like it would require a way of coordinating an additional thread, which would fit better with the possible future work of using conduit
.
Split off https://github.com/avh4/elm-format/issues/788 for possible future refactoring.
Merged via https://github.com/avh4/elm-format/pull/789
Thank you!!
Fixes https://github.com/avh4/elm-format/issues/755. Related to https://github.com/avh4/elm-format/issues/183.
On my computer, it’s roughly 2.5 times faster! :tada:
main
refers to commit 15578927a7fd327abd9c619031243c0abe96c57b.Tested on a code base of this size:
Computer specs:
Terminal:
elm-format --yes .
(without>/dev/null
) depends on your Terminal. With the default Apple Terminal, I got 3.3 seconds as shown in the table above. With iTerm2, I got 5.5 seconds (and 9.7 seconds formain
).Notes:
InfoFormatter
works. Sorry if that doesn’t fit with your vision for it!World.mapMConcurrently
(a one-liner) though, so it might be easy to replace by someone better at Haskell.