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

don't repeatedly resolve common usage paths #6123

Merged
merged 1 commit into from
May 22, 2023

Conversation

dten
Copy link
Contributor

@dten dten commented May 17, 2023

For some large packages in our project getPackageFiles has always quite a long time to complete, but it seems much worse in 9.4.x. (~7.5 seconds on my machine with stack 2.11 and ghc 9.4.5). Profiling showed it spent most of its time checking the the files from usages in .hi files existed.

When I checked what paths it was checking and how often I found that every .hi file mentioned many .so files (~150) and each of these files was being checked it existed for each .hi file.

This change attempts to collect up known resolution mappings whilst walking the .hi files so the resolution effort doesn't get repeated.

This takes our slowest getPackageFiles call down to ~3.6 seconds with the profile now showing the cost is mostly parsing the .hi files.

I've included before and after profile results where this was basically stack test --file-watch --watch-all on an already build project to get to the point where it's ready to build new changes.

before: note the wide sections calling forgivingResolveFile
stack_before

after: those wide sections are now gone
stack_after

the profiles also show stack does all the getPackageFiles work twice which isn't ideal but i haven't looked into yet 👀

@mpilgrem
Copy link
Member

@dten, many thanks. I will review, but please bear with me. When it comes to 'architectural' matters, it takes me a little while to work through proposed changes. If another potential reviewer sees this and is more up to speed with the architecture, please do take the lead.

@dten
Copy link
Contributor Author

dten commented May 18, 2023

sure thing 😃 by the way it looks likely the many .so file usings appear when using template haskell and was probably introduced to the .hi files here
https://gitlab.haskell.org/ghc/ghc/-/merge_requests/7353

@mpilgrem mpilgrem merged commit dca9768 into commercialhaskell:master May 22, 2023
12 checks passed
@mpilgrem
Copy link
Member

Looks good to me.

mpilgrem added a commit that referenced this pull request May 23, 2023
@mbj
Copy link
Contributor

mbj commented May 29, 2023

This patch moves my project from 1.5s on getPackageFiles to 0.75s. Thanks.

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

Successfully merging this pull request may close these issues.

None yet

3 participants