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

Rails 6 EventProf Empty #181

Closed
philipbjorge opened this issue Mar 11, 2020 · 5 comments · Fixed by #182
Closed

Rails 6 EventProf Empty #181

philipbjorge opened this issue Mar 11, 2020 · 5 comments · Fixed by #182

Comments

@philipbjorge
Copy link

philipbjorge commented Mar 11, 2020

Tell us about your environment

Ruby Version: 2.6.5

Framework Version (RSpec, Minitest, FactoryGirl, Rails, whatever): RSpec 3.8, Rails 6.0.2.1

TestProf Version: 0.11.3

What did you do?

Ran env EVENT_PROF_TOP=3 EVENT_PROF_EXAMPLES=1 EVENT_PROF='factory.create,perform.active_job,sql.active_record' bundle exec rspec spec/system

What did you expect to happen?

I expected to see EventProf results...

What actually happened?

Screen Shot 2020-03-11 at 7 42 01 AM

Details

When I attached and set a breakpoint here: https://github.com/palkan/test-prof/blob/master/lib/test_prof/event_prof/instrumentations/active_support.rb#L12

The result for (finish - start) was always 0.

When I changed the code to the following:

::ActiveSupport::Notifications.subscribe(event) do |event|
  yield (event.duration / 1000.0)
end

I started getting profiling results!

Code was based on: https://blog.bigbinary.com/2019/04/24/rails-6-adds-cpu-time-idle-time-and-allocations-to-activesupport-notifications-event.html

@palkan
Copy link
Collaborator

palkan commented Mar 11, 2020

Do you use something like Timecop to freeze the time?

As I see, the old style subscribers (with |_event, start, finish, *| args) use Time.now, while the new one, Evented, uses Concurrent.monotonic_time.

As I see, the difference is intentional: rails/rails@b9a9131

I think, using monotonic_subscribe instead of subscribe should fix this in Rails 6. Though for earlier versions we need a different way of dealing with patched Time.now.

@philipbjorge
Copy link
Author

We do use Timecop -- I was wondering if something like this might have been at the root of things but am just discovering this codebase.

Can I help assist further?

@palkan
Copy link
Collaborator

palkan commented Mar 11, 2020

Can I help assist further?

Thanks but I've already started working on a fix)

@palkan
Copy link
Collaborator

palkan commented Mar 12, 2020

@philipbjorge Fixed in master. Will be released soon (after we finish #178).

@philipbjorge
Copy link
Author

@palkan -- Really appreciate it!

This library just helped us shave 30-45% off of our system spec suite runtimes with potential downstream savings for other projects.

https://github.com/dequelabs/axe-matchers/issues/94

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

Successfully merging a pull request may close this issue.

2 participants