erlang:apply is faster if number of params is known at compile time #4073
Conversation
…preserve such info
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #4073 +/- ##
=======================================
Coverage 83.88% 83.88%
=======================================
Files 526 526
Lines 33181 33181
=======================================
Hits 27834 27834
Misses 5347 5347
☔ View full report in Codecov by Sentry. |
| @@ -227,7 +227,7 @@ run_hook([Handler | Ls], Acc, Params, Key) -> | |||
| hook_fn_ret() | safely:exception(). | |||
| apply_hook_function(#hook_handler{hook_fn = HookFn, extra = Extra}, | |||
| Acc, Params) -> | |||
| safely:apply(HookFn, [Acc, Params, Extra]). | |||
There was a problem hiding this comment.
It was the only place we've used safely:apply function btw :)
There was a problem hiding this comment.
I know, I could try to get rid of this module but there's an entire test-suite testing properties of this. So now this is scoped to test compilation only.
include/mongoose.hrl
Outdated
| @@ -31,6 +31,13 @@ | |||
|
|
|||
| -define(MONGOOSE_URI, <<"https://www.erlang-solutions.com/products/mongooseim.html">>). | |||
|
|
|||
| -define(APPLY_SAFELY(F), | |||
There was a problem hiding this comment.
maybe not mongoose.hrl, could be apply_sefely.hrl...
Also, it is not APPLY_SAFELY(F) anymore, it is catch_v2 :D So, maybe some better name.
There was a problem hiding this comment.
I could happily create such header and simply use the header from both source and test and kill the file.
When it comes to naming, oh naming is hard and it has always been called safely so, dunno :D
There was a problem hiding this comment.
because it was doing erlang:apply(F). Now it does F.
But yea, I would be always think it should be F(), when I would read it. And the arg should be a fun(). So, maybe renaming F is good idea.
Maybe better, idk xdd:
-define(SAFELY(Code), try Code .... end).
or
oh god MATCH_EXCEPTIONS (maybe no) xdd
And put it in safely.hrl so everything is named safely.
NelsonVides
force-pushed
the
hooks/apply_known_arity
branch
from
0758ff6 to
25fa007
Compare
NelsonVides
force-pushed
the
hooks/apply_known_arity
branch
from
25fa007 to
c47348f
Compare
|
elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / c47348f small_tests_24 / small_tests / c47348f small_tests_25 / small_tests / c47348f small_tests_25_arm64 / small_tests / c47348f ldap_mnesia_24 / ldap_mnesia / c47348f dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / c47348f ldap_mnesia_25 / ldap_mnesia / c47348f dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / c47348f dynamic_domains_mysql_redis_25 / mysql_redis / c47348f pgsql_mnesia_24 / pgsql_mnesia / c47348f dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / c47348f muc_light_http_api_SUITE:end_per_suite{error,
{{unregistering_failed,
{amount,1},
{unregistered_items,
[{{<<"_send_message_to_room_2576">>,
[{escalus_event_mgr,<0.3074.2>},
{tc_name,send_message_to_room},
{escalus_cleaner,<0.3073.2>},
{watchdog,<0.3072.2>},
{preset,"odbc_mssql_mnesia"},
{mim_data_dir,
"/home/circleci/project/big_tests/tests/muc_light_http_api_SUITE_data"},
{tc_logfile,
"https://circleci-mim-results.s3.eu-central-1.amazonaws.com/PR/4073/184573/odbc_mssql_mnesia.25.3-amd64/big/ct_run.test%408f02a02f5641.2023-08-06_14.22.57/big_tests.tests.muc_light_http_api_SUITE.logs/run.2023-08-06_14.35.47/muc_light_http_api_suite.send_message_to_room.146947.html"},
{tc_group_properties,[{name,positive},parallel]},
{tc_group_path,[]},
{data_dir,
"/home/circleci/project/big_tests/_build/default/lib/mongoose_tests/ebin/muc_light_http_api_SUITE_data/"},
{priv_dir,
"https://circleci-mim-results.s3.eu-central-1.amazonaws.com/PR/4073/184573/odbc_mssql_mnesia.25.3-amd64/big/ct_run.test%408f02a02f5641.2023-08-06_14.22.57/big_tests.tests.muc_light_http_api_SUITE.logs/run.2023-08-06_14.35.47/log_private/"},
{{saved_modules,mongooseim@localhost,<<"test type">>},
#{mod_adhoc => #{iqdisc => one_queue,report_commands_node => false},
mod_amp => #{},
mod_bosh =>
#{backend => mnesia,inactivity => 30,max_pause => 120,
max_wait => infinity,server_acks => false},
mod_cache_users =>
#{number_of_segments => 5,strategy => fifo,time_to_live => 2},
mod_carboncopy => #{iqdisc => no_queue},
mod_disco =>
#{extra_domains => [],iq...internal_mnesia_25 / internal_mnesia / c47348f mod_global_distrib_SUITE:hosts_refresher:test_host_refreshing{error,
{{trees_for_connections_present,true,[{times,100,false}],ok},
[{mongoose_helper,do_wait_until,2,
[{file,"/home/circleci/project/big_tests/tests/mongoose_helper.erl"},
{line,357}]},
{mod_global_distrib_SUITE,test_host_refreshing,1,
[{file,
"/home/circleci/project/big_tests/tests/mod_global_distrib_SUITE.erl"},
{line,384}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}}pgsql_mnesia_25 / pgsql_mnesia / c47348f mssql_mnesia_25 / odbc_mssql_mnesia / c47348f mysql_redis_25 / mysql_redis / c47348f internal_mnesia_25 / internal_mnesia / c47348f |
|
@NelsonVides please add missing PR descrition - it's not clear what the motivation for this change is. |
I know, Mikhail merged slightly faster than I thought. Writing a description now 🙏🏽 |
Now, description added 🙂 |
According to the erlang's efficiency guide, at https://www.erlang.org/doc/efficiency_guide/functions#function-calls
So giving information to the hook engine about the actual number of parameters, makes the call fall into the second category instead of the fourth. In local micro-benchmarking using benchee, it was around 2% faster.