Clarify what specifying the module name does with &

[jyc]

Hello! Thanks for making Elixir.

I ran into this error message from :telemetry and couldn't understand why it was happening, even after doing some web searches & reading the Elixir docs:

[info] The function passed as a handler with ID “…” is a local function.
This means that it is either an anonymous function or a capture of a function without a module specified. That may cause a performance penalty when calling that handler. For more details see the note in telemetry:attach/4 documentation.
telemetry — telemetry v1.2.1

After I asked on the Elixir Forums, I was pointed to a mailing list thread (https://groups.google.com/g/elixir-lang-core/c/rW_BRCULsGk/m/9rrYEwwtAwAJ 1) where someone said:

The difference comes into play with code loading. The local captures will reference the version of the module they were captured with, while remote/external captures will always reference the latest version of the module.

This is a PR to add a note about that to the docs for &. I am still new to the language so I am of course happy to move the documentation elsewhere and/or change the wording!

[sabiwara]

I think this is a bit misleading, because imported functions would also be external, it isn't about the m.f/a syntax:

iex> &to_timeout/1
&Kernel.to_timeout/1

Perhaps the point could be focused on &local_function/1 versus &__MODULE__.local_function/1?

[jyc]

Thanks for the correction! Will do

[sabiwara]

Since hot code reloading is not so common, I think it is worth making this explicit this is only applies in this context?

[jyc]

OK!

[jyc]

Thanks for your suggestion! Updated the wording, let me know any other changes I can make. I'm new to this so I'm not attached to any wording.

[sabiwara]

LGTM, thank you! 💜

Just wondering (question for the team), would an admonition block (e.g. Local versus remote captures) be clearer?

[michalmuskala]

Are we 100% sure that remote captures are faster than local captures? Could we have some benchmarks on latest versions to verify? I'd be actually quite surprised to see a difference knowing how they are implemented underneath.
There is a difference when the capture is created - remote captures are compile-time literals and local captures are load-time literals (affecting efficiency of embedding inside other literals).
There is also the semantic difference as explained.
That said, there was quite some work in recent Erlang versions around function objects, so performance characteristics might have changed since that documentation was written.

[sabiwara]

@michalmuskala yes I did a quick one for https://groups.google.com/g/elixir-lang-core/c/rW_BRCULsGk/m/GeMd5Mn4AgAJ, it showed a very small gap which should be irrelevant for most cases but does exist.

I tried to benchmark these and sometimes local functions indeed show up as slightly slower, but it is typically in the 0~2% range even for this contrived example doing basically nothing. So I'm not sure if it is worth highlighting the difference:
sabiwara/elixir_benches@9b721f3

Checking the S file shows that external functions are just literals while local functions are calling make_fun3:
sabiwara/elixir_benches@9b721f3#diff-3a3b83d8acad1d3b6faed87ba471843f4424b22c26c179bf1d86a87a880e8223R74-R93

Even though, as far as I know, recent versions of Erlang have optimizations to make sure the overhead of lambdas like this one remains very small: erlang/otp#6963.

[michalmuskala]

The benchmark linked compares the performance of both crating and using the capture. The text change in documentation talks about the performance of using them - I'm not convinced this benchmarks fully proves the conclusion presented here, specifically that, as you noticed, there is indeed an expected difference in efficiency of creating such function objects

[sabiwara]

The benchmark linked compares the performance of both crating and using the capture. The text change in documentation talks about the performance of using them - I'm not convinced this benchmarks fully proves the conclusion presented here, specifically that, as you noticed, there is indeed an expected difference in efficiency of creating such function objects

Thank you, this makes sense, let me give it another try 👍

[sabiwara]

I tried to update the benchmark, but I can't get any consistent results, every run gives different results. I think the difference is almost too small to be noticeable.

Perhaps we should reconsider documenting this? (at least the performance aspect, the hot code reload aspect might still be interesting?)

[michalmuskala]

I agree that the code reloading aspect is an important distinction and would be useful to document

[whatyouhide]

I think maybe the discussion on whether we should warn on local captures should happen in https://github.com/beam-telemetry/telemetry, and we keep this improvement here?

[josevalim]

@jyc can you please update to only mention local vs remote and their implications in regards to code reloading? Thank you.

[jyc]

Absolutely! I removed the "This has performance implications" sentence. Do you think it would make sense for me to open a PR removing the warning in beam-telemetry/telemetry?

[josevalim]

💚 💙 💜 💛 ❤️