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.

Sign up for GitHub

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

Jump to bottom

Add benchmarks for hole fits #2027

Merged
merged 4 commits into from
Jul 22, 2021
Merged

Add benchmarks for hole fits #2027

merged 4 commits into from
Jul 22, 2021

Conversation

pepeiborra
Copy link
Collaborator

Let's find out how expensive hole fits are across GHC versions and benchmark examples

@pepeiborra
Copy link
Collaborator Author

pepeiborra commented Jul 20, 2021 •
edited
Loading

The results for 8.10.4 are really quite bad. For the lsp-types examples, we have 10s total time for normal edits, vs 186s for edits with 2 holes. Roughly 18X slower. The allocations tell a similar story, 9.3GB vs 199GB.

version    name                             success   samples   startup              setup                userTime             delayedTime             totalTime            maxResidency   allocatedBytes
upstream   edit                             True      50        1.734907637          0.0                  0.60276508           10.115454337999992      10.724588409         200MB          9359MB
upstream   hole fit suggestions             True      50        1.6322459790000001   0.0                  186.34176163599994   2.3826531999999997e-2   186.37326197500002   280MB          199205MB

@pepeiborra pepeiborra marked this pull request as ready for review July 20, 2021 18:01
@pepeiborra
Copy link
Collaborator Author

Once this has landed, I'll open a new draft PR to disable refinement hole fits so that we can measure the impact and discuss.

jneira
jneira approved these changes Jul 20, 2021
View reviewed changes
Copy link
Member

@jneira jneira left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, those are really bad numbers

Ailrun
Ailrun approved these changes Jul 21, 2021
View reviewed changes
Copy link
Member

@Ailrun Ailrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Ailrun Ailrun added the merge me Label to trigger pull request merge label Jul 21, 2021
@pepeiborra pepeiborra merged commit 46eaf58 into master Jul 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jneira jneira jneira approved these changes

@Ailrun Ailrun Ailrun approved these changes

Assignees
No one assigned
Labels
merge me Label to trigger pull request merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pepeiborra @jneira @Ailrun