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

Delay loading of AliasAnalysis data #10837

Merged
merged 42 commits into from
Aug 19, 2024
Merged

Conversation

JaroslavTulach
Copy link
Member

@JaroslavTulach JaroslavTulach commented Aug 16, 2024

Pull Request Description

Continuation of #10729 and a step towards #10833 to actually speed things up by 10% by delaying loading of AliasAnalysis data.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • All code follows the
    Scala,
    Java,
  • Unit tests continue to pass.

Akirathan added 30 commits July 31, 2024 19:21
This ensure that one can see all the metadata on the IR.
# Conflicts:
#	engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PassPersistance.java
#	engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/alias/graph/Graph.scala
@JaroslavTulach JaroslavTulach added the CI: No changelog needed Do not require a changelog entry for this PR. label Aug 16, 2024
@JaroslavTulach JaroslavTulach self-assigned this Aug 16, 2024
@JaroslavTulach
Copy link
Member Author

JaroslavTulach commented Aug 16, 2024

Running stdlibs benchmarks to verify impact on startup. There is 10% speedup of the most relevant benchmark!

10% on Hello World!

That's good, but not as good as one would hope for. Running:

enso$ ./built-distribution/enso-engine-*/enso-*/bin/enso --run test/Benchmarks/src/Startup/Hello_World.enso 

one can still see more than 500 of AliasAnalysis metadata being read. Most (if not all) of them coming from EnsoRootNode.buildFrameDescriptor() - we need to find a build to build the descriptor just from FramePointers without needing AliasAnalysis.

@JaroslavTulach
Copy link
Member Author

JaroslavTulach commented Aug 17, 2024

Few functional changes were needed:

  • 32344c1 fixes ReplTest failures
  • 38bc732 - LocalScope embeds a mutable internals and it cannot be shared across multiple EnsoRootNodes.

Running stdlibs benchmarks to verify impact on startup. There is 10% speedup of the most relevant benchmark!

Another stdlibs benchmarks run to verify the state after recent changes. The 10% speedup still remains there:

still 10%

Marking ready to merge, if Pavel agrees.

@JaroslavTulach JaroslavTulach added the CI: Ready to merge This PR is eligible for automatic merge label Aug 17, 2024
Copy link
Member

@Akirathan Akirathan left a comment

Choose a reason for hiding this comment

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

Nice and small change. Great to see the stable 10% startup improvement. Let's merge this and continue with investigation of EnsoRootNode.buildFrameDescriptor() as mentioned in #10837 (comment)

@mergify mergify bot merged commit 384e019 into develop Aug 19, 2024
43 checks passed
@mergify mergify bot deleted the wip/jtulach/DelayAliasAnalysis branch August 19, 2024 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants