stdlib: optimise gen_statem by caching callback function #7419
Conversation
Inspired by f0874ff, we want to cache `Mod:handle_event/4` when such is the configured callback mode, to make calling this function faster for state machines.
CT Test Results 2 files 89 suites 40m 20s ⏱️ For more details on these failures, see this check. Results for commit 07eff3b. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
lib/stdlib/src/gen_statem.erl
Outdated
| @@ -1375,8 +1382,8 @@ loop_state_callback( | |||
| case CallbackMode of | |||
| state_functions -> | |||
| Module:State(Type, Content, Data); | |||
| handle_event_function -> | |||
| Module:handle_event(Type, Content, State, Data) | |||
| {handle_event_function, HandleEvent} -> | |||
There was a problem hiding this comment.
As there's only two options and any errors to that effect should've been caught before we got here, can't we skip the wrapping tuple for HandleEvent and just call it directly?
There was a problem hiding this comment.
More than happy to do it that way ofc, just let me know if I should – asking because Raimo's been tagged so maybe we want his opinion first (?)
Implement not wrapping the fun() for handle_event_function.
Fix the handle_event_fun() type. The suggestion's type
was that the handle_event_fun() should be either
a state enter fun() or a handle event fun(),
but it should be a fun() that is both.
Revert the callback_mode/1 type checking function and introduce
params_callback_mode/2 to do the conversion into the cached value.
By that most of callback_mode_result/6 could be reverted,
only the finishing #params{} construction needs a change.
|
I had opinions, and the most explicit way to convey them was to rework your suggestion ;-) This resulted in a smaller diff. What do you think? |
|
That looks very neat to me, thanks for the suggestions and the notes :) |
|
Thank you for your pull request! |
Inspired by #5831 (f0874ff), we want to cache
Mod:handle_event/4when such is the configured callback mode, to make calling this function faster for state machines.I'm not sure of the names of the newly defined types though, better names are welcomed. The idea is to cache the callback together with the statem params where we store the callback mode.