Skip to content

Uses strict Map to fix a compile time regression#4521

Merged
rhendric merged 4 commits intopurescript:masterfrom
mjrussell:mjr/fix-compilation-time-regression
Nov 24, 2023
Merged

Uses strict Map to fix a compile time regression#4521
rhendric merged 4 commits intopurescript:masterfrom
mjrussell:mjr/fix-compilation-time-regression

Conversation

@mjrussell
Copy link
Copy Markdown
Contributor

@mjrussell mjrussell commented Nov 24, 2023

Description of the change

For extremely large files (14K lines) with a lot of types and instances memory increases dramatically using Lazy Maps, causing swapping and an big increase in compilation time.

Switching to a strict map brings compilation performance close to 15.2 levels.

Fixes #4491


Checklist:

  • Added a file to CHANGELOG.d for this PR (see CHANGELOG.d/README.md)
  • Added myself to CONTRIBUTORS.md (if this is my first contribution)
  • Linked any existing issues or proposals that this pull request should close
  • Updated or added relevant documentation
  • Added a test for the contribution (if applicable)

For extremely large files (14K lines) with a lot of types and instances
memory increases dramatically using Lazy Maps, causing swapping and an
big increase in compilation time.

Switching to a strict map brings compilation performance close to 15.2
levels.

Fixes purescript#4491
@rhendric
Copy link
Copy Markdown
Member

Would you mind adding an entry for this in CHANGELOG.d? (See CHANGELOG.d/README.md for guidance.)

Otherwise LGTM, assuming the build raises no issues! And thanks for ferreting this out.

@mjrussell
Copy link
Copy Markdown
Contributor Author

Would you mind adding an entry for this in CHANGELOG.d? (See CHANGELOG.d/README.md for guidance.)

Done!

then env{ _depth = depth', _deepestTopLevelScope = depth' }
else env{ _depth = depth' }
where
where
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this change caused by whitespace? If so, can that be undone?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah there was a trailing whitespace my editor deleted, I can undo that if you want to keep it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I say leave it; ordinarily I'd want to keep changes like this from distracting from the diff but this diff is tiny and the change is very unobjectionable.

@JordanMartinez
Copy link
Copy Markdown
Contributor

Looks like the ARM builds are not starting again.

@f-f
Copy link
Copy Markdown
Member

f-f commented Nov 24, 2023

Having a look - @JordanMartinez feel free to tag me when this happens

@f-f
Copy link
Copy Markdown
Member

f-f commented Nov 24, 2023

They are back up 👍

@rhendric rhendric merged commit a915253 into purescript:master Nov 24, 2023
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.

Regression in compilation time and memory in Purescript > 0.15.2

4 participants