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

Optimize WithArray arr (\v -> body) #140

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

emwap
Copy link
Member

@emwap emwap commented Jan 29, 2015

Substitute arr for v when arr is free in the body.

This is very nice for copy propagation, but not possible in the typed
representation.

Is this rewrite morally safe? I.e. is it enough to check that arr is free in the body or do we risk leaking an alias to arr?

/cc @pjonsson, @emilaxelsson, @josefs

Substitute `arr` for `v` when `arr` is free in the `body`.
This is very nice for copy propagation, but not possible in the typed
representation.
@josefs
Copy link
Contributor

josefs commented Jan 29, 2015

Clarification from mine and Anders' discussion: the variable arr does not occur in body.

This is morally safe in single threaded code if body does not return the array (which may be hard to check).

@pjonsson
Copy link
Member

The point of @josefs clarification was what I was after in my previous comment in the other pull request as well. That should be fixed in the comment in this commit as well.

Mutable is really just a different word for "very hard", especially in the context of concurrency. It sounds reasonable that this is safe in a single threaded context. I have no idea whether we are guaranteed to be in a single threaded context at all times though.

Substitute `arr` for `v` when `arr` is not used in the `body`.
This is very nice for copy propagation, but not possible in the typed
representation.
@emilaxelsson
Copy link
Member

Just a note: Peter has been talking about type checking the simplified code as a way to catch errors. Maybe there should be an unsafeFreezeArray to make rewrites like this pass the type checker (and then it could be done in the typed representation as well).

@josefs
Copy link
Contributor

josefs commented Jan 31, 2015

Another possibility is to do this kind of rewrite at a lower level representation, i.e. the imperative representation. At that point there is no distinction between mutable and immutable arrays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants