-
Notifications
You must be signed in to change notification settings - Fork 323
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
Remove TailCall.TailPosition.NotTail metadata. #11303
Conversation
If IR element is not in tail position, there is simply no metadata.
private def updateMetaIfInTailPosition[T <: IR]( | ||
isInTailPosition: Boolean, | ||
ir: T | ||
): T = { | ||
if (isInTailPosition) { | ||
ir.updateMetadata(TAIL_META) | ||
} else { | ||
ir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the gist of this PR. There is no NotTail
attached anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for extracting the discovery you made while working on @Tail_Call
mini passes and applying it for the benefit of size of .bindings
caches. Have you measured the effect on the size of the caches? I'd assume it is at least five bytes per each non-tail statement. E.g. it might be a lot.
/** The expression is not in a tail position and cannot be tail call | ||
* optimised. | ||
*/ | ||
final case object NotTail extends TailPosition { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no NotTail
anymore. TailPosition
has only one instance and that's the case object Tail
.
engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PassPersistance.java
Outdated
Show resolved
Hide resolved
@JaroslavTulach I have not measured it. If you are interested, I will do the measurements, but for me, it is enough to know that it will definitely be smaller. However, I don't think the change will be that dramatic, because previously, the
|
engine/runtime-compiler/src/main/java/org/enso/compiler/pass/analyse/PassPersistance.java
Show resolved
Hide resolved
In general I see 5% decrease in
|
Pull Request Description
TailCall.TailPosition.NotTail
metadata is attached to everyIR
element that is not in tail position. Which is true for most IR elements. This is inefficient and unnecessary. This PR removes this metadata and keeps onlyTailCall.TailPosition.Tail
. This removes few bytes fromMetadataStorage
from most of IR elementsImportant Notes
To check whether
ir
element is not in tail position, we used to check it with something like thisir.getMetadata(TailCall) == Some(TailCall.TailPosition.NotTail)
, now, we simply can do it withir.getMetadata(TailCall) == None
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.