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

Split syntax module to reduce compilation time #1852

Merged
merged 5 commits into from
May 18, 2024

Conversation

nitinprakash96
Copy link
Collaborator

@nitinprakash96 nitinprakash96 commented May 17, 2024

Hello,
Splitting Swarm.Language.Syntax into submodules reduces the compilation time quite a bit. Currently, it takes the most of time out of all the modules. Here's a chart:

CleanShot 2024-05-17 at 17 09 38@2x

That's ~20s for simplifier (with overall compilation time of 4.3 mins)

Just taking out the types and functions related to Constants and putting them in a separate module results in:

CleanShot 2024-05-17 at 17 11 45@2x

Thats ~8.5 seconds for simplifier (with overall compilation time of 3.9 mins)

Just for added info, timings taken on a 2019 macbook with i7 processor (6 core and 32 GB RAM)

Changes include:

  • Swarm.Language.Syntax split into the following modules:
    • Swarm.Language.Syntax.Comments - Types for working with comments.
    • Swarm.Language.Syntax.Type - Core types (Syntax', Term' and DelayType)
    • Swarm.Language.Syntax.Pattern - Pattern synonyms for untyped terms
    • Swarm.Language.Syntax.Loc - Types for working with location in source code (SrcLoc and related types)
    • Swarm.Language.Syntax.Util - Helper functions (Eg: mkTuple, freeVarsS etc)

Compilation chart after the split:

CleanShot 2024-05-18 at 15 20 17@2x

Swarm.Language.Syntax is no longer the most time taking module.

Closes #1844

@byorgey
Copy link
Member

byorgey commented May 17, 2024

Hi @nitinprakash96, thanks, this is fantastic!

This PR is just an initial stab at it. If you're accepting patch for the issue #1844, I'm happy to split the module in question further accordingly.

For sure, if you'd like to work on splitting it up further that would be great. I'm imagining that in the end the Swarm.Language.Syntax module itself would only exist to re-export stuff from Swarm.Language.Syntax.*.

If you click on "Details" next to the failed 'restyled' check, it will take you to a page with instructions on how to apply the restyling. Alternatively you can run fourmolu yourself. But you can wait to deal with that until the PR is done.

@nitinprakash96
Copy link
Collaborator Author

nitinprakash96 commented May 18, 2024

I've made further splits and added top level comments (see description box for the changes). However, I'm tempted to move Swarm.Language.Directions to Swarm.Language.Syntax.Directions since it's re-exported by Swarm.Language.Syntax. But I don't know if that is desirable or not?

@byorgey
Copy link
Member

byorgey commented May 18, 2024

I think moving Swarm.Language.Direction to Swarm.Language.Syntax.Direction makes sense. It probably got split out from Swarm.Language.Syntax in the first place (a long time ago).

src/swarm-lang/Swarm/Language/Syntax/Comments.hs Outdated Show resolved Hide resolved
src/swarm-lang/Swarm/Language/Syntax/Type.hs Outdated Show resolved Hide resolved
@byorgey byorgey added the merge me Trigger the merge process of the Pull request. label May 18, 2024
@mergify mergify bot merged commit 8ad4efe into swarm-game:main May 18, 2024
11 checks passed
@byorgey
Copy link
Member

byorgey commented May 18, 2024

@nitinprakash96, congrats on having your first PR accepted to Swarm! We appreciate the contribution and we're really glad to welcome you as part of the community. In a minute we will send you an invite to the repo (see https://github.com/swarm-game/swarm/blob/main/CONTRIBUTING.md#i-have-push-access-to-the-swarm-repository-now-what ). If you use IRC, feel free to also join the #swarm channel on Libera.Chat.

@nitinprakash96 nitinprakash96 deleted the nitin/split-syntax branch May 18, 2024 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Split Swarm.Language.Syntax into more modules
2 participants