-
Notifications
You must be signed in to change notification settings - Fork 82
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
Extend measure to allow optional function args #7
Conversation
Erlang's `:timer` module already has a function for timing the duration of a function. By using it, we can also enable passing arguments to the passed-in function. Example: measure("my_key", &Enum.map/2, [my_list, mapper_func]) This reports the duration of the map function, and returns the computed value. Optionally, we could also add a `measure/4` function which takes a key, module, function, and args. See http://erlang.org/doc/man/timer.html#tc-1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch @CJPoll.
Let's skip addition of args
in that PR.
Same result can be achieved without them.
Also, since these args
will be optional, addition of options, which are optional as well, could be problematic: users will need to always provide args
if they pass options.
@@ -44,12 +44,12 @@ defmodule Statix do | |||
for pipelining and easily wrapping existing code. | |||
""" | |||
# TODO: Use `:erlang.monotonic_time/1` when we depend on Elixir ~> 1.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove this TODO altogether.
def measure(key, fun, args \\ []) when is_function(fun) do | ||
{time, result} = :timer.tc(fun, args) | ||
|
||
elapsed_ms = div(time, 1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move this calculation into the timing call:
{elapsed, result} = :timer.tc(fun)
timing(key, div(elapsed, 1000))
result
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing
I'm a bit unsure what you mean in your overall feedback about options and args conflicting? Is that in regards to the idea of a
def measure(key, module, function, args) when is_atom(module) and is_atom(function) and is_list(args) do
{time, result} = :timer.tc(module, function, args)
timing(key, div(time, 1000))
result
end Though you're right - Edited original post to include multiple examples. |
Sorry, my comment is indeed not clear enough. measure("my_key", fn -> Enum.map(my_list, mapper_func) end) Also it won't compose nicely with measure("my_key", fn -> ... end, [], [sample_rate: 0.5]) |
Ah - I wasn't aware of the other PR. That does make things difficult. Adding the args was the use case I was making this PR for. I suppose my apps can just define their own function which is equivalent to I'll push up the changes later today, then. |
I believe that should remove the problem. My apologies for the delay. |
Merged. Thank you @CJPoll. 💛 |
Erlang's
:timer
module already has a function for timing the durationof a function. By using it, we can also enable passing arguments to the
passed-in function.
Examples:
measure("my_key", &Enum.map/2, [my_list, mapper_func])
measure("my_key", fn -> Enum.map(my_list, mapper_func) end) # Maintains backwards compatibility
This reports the duration of the map function, and returns the computed
value.
Optionally, we could also add a
measure/4
function which takes a key,module, function, and args.
measure("my_key", Enum, :map, [my_list, mapper_func])
See http://erlang.org/doc/man/timer.html#tc-1