-
Notifications
You must be signed in to change notification settings - Fork 173
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
feat: stack-allocated lambdas #1368
base: master
Are you sure you want to change the base?
Conversation
Very cool, I've been waiting for this, I think some of my programs can be alloc-free now. |
Pulled the PR to play with it, found a couple of problems: The first one might be linked to #1317, is that you can return a (Ref Fn) meaning that what that Fn capture would be out of scope by the time you use the Lambda. Example: (defn gen []
(let [s1 @"Wow"
lam (fn [] (IO.println &s1))]
lam))
(defn main []
(let [lam (gen)]
(~lam))) The second one is related to the Lambda deleter which assumes what's in it's env is owned by the Lambda (which sounds correct) so it deletes things, but it's not actually taking ownership properly so the normal deleters for the variables still run when the scope ends. So you end up with a double free. Example: (defn gen []
(let [s1 @"Wow"
s2 @"Lambda"
lam (fn [] (IO.println &(String.join "" &[s1 s2])))]
(~lam)))
(defn main []
(gen)) |
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.
Very nice! I'm wondering, if one should need a malloc
'd lambda for some reason--would it be possible after this change? How would one do that? Not sure if it'd ever be necessary though.
@@ -327,7 +327,7 @@ mkLambda visited allowAmbig _ tenv env root@(ListPat (FnPat fn arr@(ArrPat args) | |||
modify (deleterDeps ++) | |||
modify (copyFn :) | |||
modify (copyDeps ++) | |||
pure (Right [XObj (Fn (Just lambdaPath) (Set.fromList capturedVars)) (xobjInfo fn) (xobjTy fn), arr, recBody]) | |||
pure (Right [XObj (Fn (Just lambdaPath) (Set.fromList capturedVars)) (xobjInfo fn) (xobjTy fn), arr, body]) |
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.
Was the change from recBody
to body
intentional? I think this was needed to appropriately handle recursive anonymous functions in let bindings, e.g. (let [f (fn [x] (if (= x 0) x (f (- x 1)))])
, since we rename the lambda (see renameRecursives above). It doesn't seem like these changes would alter that behavior but maybe I'm missing something.
@scolsen The idea is that you copy the lambda and that will put its environment on the heap, just like before. |
Yes, this seems to be working, it's just that it's hitting the double free problem from above: (defn gen []
(let [s1 @"Wow"
s2 @"Lambda"
lam (fn [] (IO.println &(String.join "" &[s1 s2])))]
@lam))
(defn main []
((gen))) |
@TimDeve will look into that asap! |
This makes
(fn [...] ...)
change type from(Fn [...] ...)
to(Ref (Fn [...] ...)
, making it possible to avoid any dynamic memory allocations (malloc
) unless the lambda has to be copied.For example, this simple case becomes completely static:
In the case where the lambda captures managed types, those are deleted at end of their scope and are never handled by the lambda (since the lambda, being a ref, never calls it's own deleter). Here's a somewhat contrived example:
This feature introduces some (more) cases where a lambda can refer to a dead reference without a compiler error, these must be fixed by the more complete solution described in #1317. I still think it's worthwhile to merge this, since it allows for much more efficient programs (and I plan to tackle the improved lifetimes next anyways.)