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 named funs and fun-wrapped macros #1973

Conversation

@jhogberg
Copy link
Contributor

@jhogberg jhogberg commented Oct 2, 2018

This PR optimizes local fun usage by calling their implementation directly when possible, avoiding the creation of fun objects. This greatly speeds up macros that wrap their body in a fun, as well as "named funs" whose current implementation requires them to recreate themselves when used, even for something as simple as recursion.

%% make_fun2 + call_fun is now a simple call in the two funs below.

(fun(Name) -> io:format("Hello ~s!", [Name]) end)("Steve").

fun Seq(_Term, 0) -> [];
    Seq(Term, N) -> [Term | Seq(Term, N - 1)]
end.

https://bugs.erlang.org/browse/ERL-639

We forgot to do this in the BSM optimization branch, and this is
as good a time as any to get it fixed.
@jhogberg jhogberg self-assigned this Oct 2, 2018
@josevalim
Copy link
Contributor

@josevalim josevalim commented Oct 2, 2018

@jhogberg this is very exciting as we could leverage this in a few places! 🎉

Just to clarify, will the optimization apply even if the function is used over multiple branches but never escaped? Take this nonsensical code just as an example:

Fun = fun(X) -> erlang:display({value, X}) end,
case SomeCondition of
  true -> Fun(Value), ok;
  false -> Fun(Value), error
end.

Follow up question, does it also apply for function captures such as fun some_private/2? If it does, I believe we need to make sure to inline the call to some_private/2 and not the generated intermediate function as HiPE wouldn't like if the intermediate function is called both directly and via an anonymous function.

@jhogberg
Copy link
Contributor Author

@jhogberg jhogberg commented Oct 2, 2018

Just to clarify, will the optimization apply even if the function is used over multiple branches but never escaped? Take this nonsensical code just as an example:

Yep. The optimization is completely turned off if it's escapes however, and it makes no attempt to delay fun creation until needed. This would be relatively straightforward to do but we haven't found a practical use for it yet and there are a few wrinkles where it could generate worse code.

Follow up question, does it also apply for function captures such as fun some_private/2? If it does, I believe we need to make sure to inline the call to some_private/2 and not the generated intermediate function as HiPE wouldn't like if the intermediate function is called both directly and via an anonymous function.

Yes, it applies to anything that creates local funs. I'll look into that, thanks for pointing it out!

@nox
Copy link
Contributor

@nox nox commented Oct 2, 2018

Cf. 42b87be which removes the abstraction @josevalim mentions but unfortunately breaks HiPE.

@nox
Copy link
Contributor

@nox nox commented Oct 2, 2018

11:56 <josevalim> nox: in case you want to link to the whole diff: master...nox:rm-reverse-eta-conversion

I was too lazy to find my own patch.

@jhogberg jhogberg force-pushed the jhogberg:john/compiler/improve-named-funs/OTP-15273/ERL-639 branch 2 times, most recently from 3378eed to f22cd26 Oct 2, 2018
@jhogberg
Copy link
Contributor Author

@jhogberg jhogberg commented Oct 2, 2018

As far as I can tell the problems with HiPE would only occur when mixing make_fun and direct calls, which this pass avoids at the moment. Local funs always have their own intermediate function (even when wrapping another private function as below), and if the make_fun instruction disappears then there is no way for it to be referenced by both a fun and a regular call.

private() -> ok.
foo() ->
    A = fun private/0, %% '-foo/0-fun-0-'
    B = fun private/0, %% '-foo/0-fun-1-'
    Res = A(),
    Res = B().

Chances are I'm wrong though. Do you have any sneaky test cases to share from the last time this was an issue?

I've pushed an update that short-circuits direct calls to intermediate functions as it's a neat optimization in its own right, and we'll need it anyway when expanding this pass. Thanks again for pointing this out!

@nox
Copy link
Contributor

@nox nox commented Oct 2, 2018

Local funs always have their own intermediate function

That's what my patch tried to remove years ago, and HiPE was the only thing blocking that removal.

@jhogberg
Copy link
Contributor Author

@jhogberg jhogberg commented Oct 2, 2018

That's what my patch tried to remove years ago, and HiPE was the only thing blocking that removal.

Indeed. HiPE should add these intermediate functions when needed instead of imposing restrictions on the compiler, .beam may be a stable format but the generated code is not.

@jhogberg jhogberg force-pushed the jhogberg:john/compiler/improve-named-funs/OTP-15273/ERL-639 branch from f22cd26 to f630de6 Oct 3, 2018
@jhogberg jhogberg requested a review from bjorng Oct 3, 2018
@bjorng
bjorng approved these changes Oct 3, 2018
If a fun is defined locally and only used for calls, it can be replaced
with direct calls to the relevant function. This greatly speeds up "named
functions" (which rely on make_fun to recreate themselves) and macros that
wrap their body in a fun.
@jhogberg jhogberg force-pushed the jhogberg:john/compiler/improve-named-funs/OTP-15273/ERL-639 branch from f630de6 to 31a4c1d Oct 3, 2018
@jhogberg jhogberg merged commit a4c2d23 into erlang:master Oct 4, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
@josevalim josevalim mentioned this pull request Mar 27, 2019
0 of 10 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants