Proposal: Add return value to skip update in Access.get_and_update/3 callback

[eksperimental]

I have noticed that there are times that based on the get value, there
is no need to update the data, and we have just to pass the same data
back.
So in addition to `:pop` I propose to add `:keep` or `:skip`.

This will optimize things quiet a not only in {Map,
Keyword}.get_and_update/3 but aslo in Access.get_and_update/3 since we
won't have to dispatch and return directly.

Please let me know what you think,
Thank you.

[José Valim]

Hi Eksperimental,

Thanks for the proposal!

Can you please provide an example of where you think this would be optimized? I am certain for keyword it won't make a large difference because the cost is in traversing the keyword. And for maps, usually there is no update if you are trying to put the same value back, so I would say those concerns are already handled within the runtime.

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/605485c2.1c69fb81.df633.535dSMTPIN_ADDED_MISSING%40gmr-mx.google.com.

[Rich Cavanaugh]

Hi José,

  Separate from any performance concerns, I’d say it’s a minor change that vastly improves clarity as it relates to the intention of the code. When reviewing code I’d certainly have to read more carefully seeing something equivalent to `{current_value, current_value}` than I would `:keep`

rich

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAGnRm4JqLTQ%3DNm%3Db2rvdrY93FT%3DqOTNCcGi%3DpksK%2BgsP5EPe9w%40mail.gmail.com.

[eksperimental]

On Fri, 19 Mar 2021 04:43:25 -0700
Rich Cavanaugh <rcava...@gmail.com> wrote:

> When reviewing code I’d certainly have to read more carefully seeing
> something equivalent to `{current_value, current_value}` than I would
> `:keep`

Yes, this is also a very good point.

On Fri, 19 Mar 2021 12:30:41 +0100
José Valim <jose....@dashbit.co> wrote:

> Can you please provide an example of where you think this would be
> optimized? I am certain for keyword it won't make a large difference
> because the cost is in traversing the keyword. And for maps, usually
> there is no update if you are trying to put the same value back, so I
> would say those concerns are already handled within the runtime.

I had to implement it to see it. I knew it would be optimised for
keyword lists, because we would be saving reversing the accumulator when
then list is long and the key is at the end of the list (worst case
scenario).

The results took me by surprise for Maps.
Benchmark results.
- map:
https://gist.github.com/eksperimental/77d5c67427fed0a8a9e7eee21219febb
- keyword:
https://gist.github.com/eksperimental/1bac29e80fd8329940be2cc8e6b86d52

This is the repository for the benchmarks
https://github.com/eksperimental/benchmark_keep

and this is the implementation in Elixir that passes all the tests,
https://github.com/eksperimental/elixir/tree/keep

[eksperimental]

> Benchmark results.
> - map:
> https://gist.github.com/eksperimental/77d5c67427fed0a8a9e7eee21219febb
> - keyword:
> https://gist.github.com/eksperimental/1bac29e80fd8329940be2cc8e6b86d52
>
> This is the repository for the benchmarks
> https://github.com/eksperimental/benchmark_keep

I forgot to mention that the benchmarks measure the worst case which is
get and update the last key in maps and keywords according to the
Erlang term ordering.

[José Valim]

Thanks for the benchmarks. It turns out that for large maps we are always doing the update, I will try to send a PR upstream.

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/6058b5de.1c69fb81.f7fcd.71fcSMTPIN_ADDED_MISSING%40gmr-mx.google.com.