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

add some metrics to the "hooks_type_test" module #3554

Closed
wants to merge 1 commit into from

Conversation

fdie
Copy link
Contributor

@fdie fdie commented Mar 21, 2021

I found "hooks_type_test" very usefull and added some metrics to evaluate the performance of the various modules on a runnning system.
I don't know all the usages of "hooks_type_test", but here is how I use it, maybe helpfull for others ?
I generally grab some hook call with redbug :

erl> redbug:start("mod_mam:user_send_packet").

then define the args of the trace

erl> Acc = {{message,...},#{...}}.  % copy paste the args of the call

To feed in the associated hook method in hooks_type_test to get some execution timings:

erl> hooks_type_test:user_send_packet(Acc).
{
   {{message,...}, #{...}},
   % added map with detailed metrics and totals
   #{
      measures => [
       "{microseconds, reductions, {module, function}}",
        {2,14,{mod_privacy,user_send_packet}},
        {1,12,{mod_caps,user_send_packet}},
        {34,335,{mod_mam,user_send_packet}},
        {12,68,{mod_mam,user_send_packet_strip_tag}},
        {1,13,{mod_vcard_xupdate,user_send_packet}},
        {error,14,{mod_service_log,log_user_send}},
        {1,12,{mod_privilege,process_presence_out}},
        {44,28,{mod_ping,user_send}},
        {1,12,{mod_multicast,user_send_packet}},
        {error,12,{mod_metrics,user_send_packet}},
        {121,266,{mod_carboncopy,user_send_packet}}],
      total_microseconds => 217,
      total_reductions => 786
   }
}

I may be wrong, but I don't think this module is used in some specific context,
thus it should be possible to modifiy the return values without breaking anything.
If I'm wrong the durations may be rather sent to the console to restore the former return value.

I kept the previous version with all the explicit method calls as commented out code.

Here is a sample of the generated code:

%% called at ejabberd_c2s.erl:923
roster_get(Acc0, A) ->
    %% Acc1 = mod_roster:get_user_roster(Acc0, A),
    %% Acc2 = mod_shared_roster_ldap:get_user_roster(Acc1, A),
    %% Acc3 = mod_shared_roster:get_user_roster(Acc2, A),
    %% Acc3.
    run_metrics([Acc0, A], [
          {mod_roster, get_user_roster}
         ,{mod_shared_roster_ldap, get_user_roster}
         ,{mod_shared_roster, get_user_roster}
    ]).

Always run metrics twice, sometimes you may measure without/with caching ;-)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 35.048% when pulling 444e08d on fdie:some_hook_metrics into 453d504 on processone:master.

@Neustradamus
Copy link
Contributor

@prefiks: What do you think?

@badlop
Copy link
Member

badlop commented Mar 22, 2022

@fdie , did you see #3756 ?

@Neustradamus
Copy link
Contributor

@fdie: Have you seen the @badlop comment?

@Neustradamus
Copy link
Contributor

@fdie: Have you looked previous comments?

@badlop badlop added the To reconsider Interesting feature, but nobody offered to implement on the short term. label Mar 27, 2023
@badlop badlop added this to the Parking Lot milestone Mar 27, 2023
@badlop badlop closed this Mar 27, 2023
@Neustradamus
Copy link
Contributor

@fdie: Have you seen the @badlop comment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
To reconsider Interesting feature, but nobody offered to implement on the short term.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants