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

Is it by design that StackProf.run won't write results in ensure? #139

Open
marcandre opened this issue Jul 5, 2020 · 1 comment
Open

Comments

@marcandre
Copy link

  1. StackProf is amazing, thank you!

I hope this will be useful, but if not feel free to close this issue.

  1. My first attempt StackProf was:
def with_profiling
  return yield unless profile?

  StackProf.run(out: 'tmp/stackprof.dump') do
    yield
  end
end

That doesn't work, because StackProf.run returns the written file. I didn't expect that. I don't think it's even documented anywhere. I'm not sure it's useful. I'm not talking about the case where out: is not given, case where I understand the return value is a Hash with the results which is clearly useful.

  1. Given that I don't care about the result of StackProf.run but I do care about the result of the block, I wrote:
def with_profiling
  return yield unless profile?

  StackProf.run(out: 'tmp/stackprof.dump') do
    return yield
  end
end

Yes, I'm that lazy 😅

And no, it doesn't work, because the writing of the file is not in the "ensure" block. Is that by design? Why? Seems like a bug to me.

Of course, I can go around by doing the quite pedestrian:

def with_profiling
  return yield unless profile?

  result = nil
  StackProf.run(out: 'tmp/stackprof.dump') do
    yield
  end
  result
end

This is imo so ugly that I preferred using start and stop/results in my own ensure...

I found equally surprising that results() returns a Hash with all the results and that results('path') writes the file and returns that file (also couldn't find documentation on this) and that there is no dump_results method or similar.

@tmm1
Copy link
Owner

tmm1 commented Jul 5, 2020

I agree this looks like a bug and the results should be written within the ensure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants