Some more performance changes #236
Conversation
MangoIV
commented
Aug 17, 2024
MangoIV
commented
- max residency is down by ~ 50%
- removed two space leaks that only show up on very large files
- speed is approximately the same
- might be that it's even faster on really large files like hackage-packages.nix but I didn't run it often enough on to see
- did some refactorings
- fix a couple of space leaks - move all the extensiosn to cabal file - refactor some explictly recursive functions
|
this branch on maintainers.nix this branch on hackage-packages.nix (1 min 52 sec)
origin/master on hackage-packages.nix (5 min 17 sec) no changes in test suite times afaict |
|
I'm actually still thinking about the |
|
|
|
problem is that it probably fucks up But then again, Vector, too, and they don't even have pattern matching because it's such a bad idea... Maybe we should just rewrite the functions to something more idiomatic, at once... |
|
|
| . Megaparsec.parse Parser.file filename | ||
| where | ||
| -- f !x = layout $ maybe () (error . show) (unsafeNoThunks x) `seq` x | ||
| f !x = layout x |
There was a problem hiding this comment.
I’m still a bit experimenting that’s why I converted back to draft. The function that’s commented out checks that some closure doesn’t contain thunks :)
| unexpandSpacing' (Just n) _ | n < 0 = Nothing | ||
| unexpandSpacing' _ [] = Just [] | ||
| unexpandSpacing' n (txt@(Text _ _ _ t) : xs) = (txt :) <$> unexpandSpacing' (n <&> subtract (textWidth t)) xs | ||
| unexpandSpacing' n (Spacing Hardspace : xs) = (Spacing Hardspace :) <$> unexpandSpacing' (n <&> subtract 1) xs | ||
| unexpandSpacing' n (Spacing Space : xs) = (Spacing Hardspace :) <$> unexpandSpacing' (n <&> subtract 1) xs | ||
| unexpandSpacing' n (Spacing Softspace : xs) = (Spacing Hardspace :) <$> unexpandSpacing' (n <&> subtract 1) xs | ||
| unexpandSpacing' n (Spacing Break : xs) = unexpandSpacing' n xs | ||
| unexpandSpacing' n (Spacing Softbreak : xs) = unexpandSpacing' n xs | ||
| unexpandSpacing' _ (Spacing _ : _) = Nothing | ||
| unexpandSpacing' n ((Group _ xs) : ys) = unexpandSpacing' n $ xs <> ys | ||
| unexpandSpacing' _ Seq.Empty = Just [] | ||
| unexpandSpacing' mn (x :<| xs) | ||
| | Just n <- mn, n < 0 = Nothing | ||
| | otherwise = | ||
| let unexpandSubtract t = unexpandSpacing' (subtract t <$> mn) | ||
| in case x of | ||
| txt@(Text _ _ _ t) -> (txt :<|) <$> unexpandSubtract (textWidth t) xs | ||
| (Spacing Hardspace) -> (Spacing Hardspace :<|) <$> unexpandSubtract 1 xs | ||
| (Spacing Space) -> (Spacing Hardspace :<|) <$> unexpandSubtract 1 xs | ||
| (Spacing Softspace) -> (Spacing Hardspace :<|) <$> unexpandSubtract 1 xs | ||
| (Spacing Break) -> unexpandSpacing' mn xs | ||
| (Spacing Softbreak) -> unexpandSpacing' mn xs | ||
| (Spacing _) -> Nothing | ||
| ((Group _ ws)) -> unexpandSpacing' mn $ ws <> xs |
There was a problem hiding this comment.
This also changes the semantics a bit, because unexpandSpacing' (Just -1) Seq.Empty now returns Just [] instead of Nothing. This is likely the cause behind the changes in infinixbot/nixpkgs@2fb6f2c. Even if that would be better, let's have this PR only be internal refactorings.
There was a problem hiding this comment.
Sure, this was an accident, nice catch!
| if p /= [] && (isSoftSpacing (head p) || isSoftSpacing (last p)) | ||
| then error $ "group should not start or end with whitespace, use `group'` if you are sure; " <> show p | ||
| else p | ||
| case pretty x of | ||
| ((hp :<| _) :|> lp) | ||
| | isSoftSpacing hp || isSoftSpacing lp -> | ||
| error $ "group should not start or end with whitespace, use `group'` if you are sure; " <> show p | ||
| _ -> p |
There was a problem hiding this comment.
This also changes semantics a bit, since the first pattern only applies with at least 2 elements, whereas before it also applied with just one.
There was a problem hiding this comment.
Oh you’re right. It was sure a bit late 🙃
|
Thanks for the review. I’ll apply fixes when I get back to this ❤️ |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/formatting-team-meeting-2024-08-20/50885/1 |
