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

Track initializer's loaded event per file #12859

Merged
merged 1 commit into from
Nov 16, 2013

Conversation

pftg
Copy link
Contributor

@pftg pftg commented Nov 11, 2013

Proposed implementation of tracking load initializers for #12745

@xaviershay, you may cherry-pick this commit if you needed it for your implementation.

@@ -645,6 +645,12 @@ def routes? #:nodoc:

protected

def load_initializer(initializer)
ActiveSupport::Notifications.instrument('load_initializer.railties', initializer: initializer) do |_|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need to accept the _ variable?

@pftg
Copy link
Contributor Author

pftg commented Nov 11, 2013

Thanks @egilburg

@rafaelfranca
Copy link
Member

@pftg don't forget the CHANGELOG entry. Also, who users would use this in their applications?

@pftg
Copy link
Contributor Author

pftg commented Nov 11, 2013

@rafaelfranca, this feature good for profiling tools, to find bottlenecks in booting app.

This specific commit is help for @xaviershay, for his PR #12745, that's why I have not added changelog.

@rafaelfranca
Copy link
Member

Yes, I think for what it is good, I want to know how to use 😄 Right now there is no information. Should I add an initializer to listen the notification? Should I add a ruby script? Should I put this code somewhere else?

Of course this need to be documented.

@pftg
Copy link
Contributor Author

pftg commented Nov 11, 2013

I see, I'll add this

@xaviershay
Copy link
Contributor

Thank you! Going to try and look at this this week.

@dasch
Copy link
Contributor

dasch commented Nov 12, 2013

This is great! I would love to have this. It's pretty easy to subscribe to the event in order to get profiling info:

# development.rb
ActiveSupport::Notifications.subscribe("load_initializer.railties") do |*args|
  event = ActiveSupport::Notifications::Event.new(*args)
  puts "Loaded initializer #{event.payload[:initializer]} (#{event.duration}ms)"
end

@rafaelfranca
Copy link
Member

Needs the doc update still

@pftg
Copy link
Contributor Author

pftg commented Nov 14, 2013

@rafaelfranca will do it today/tomorrow

@rafaelfranca
Copy link
Member

👍

@pftg
Copy link
Contributor Author

pftg commented Nov 16, 2013

@rafaelfranca I added changelog with examples of exploiting this instrument.
Have not found another places where should be added docs for examples.

end

# config/initializers/benchmark_initializers.rb
ActiveSupport::Notifications.subscribe('load_initializer.railties') do |*args|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is weird to me to put this code in an initializer. It is subscribing the load event of initializer and it will generate data for this event. I think we should recommend to put in the initializers folder

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, I'd just take out this example. There are load ordering issues as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:) just show possibility, that you can subscribe even in initializers. About order, I forget mention about naming of initializer. Elsewhere I'll remove!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two examples is plenty :)

In order to simplify profiling loading of initializers,
added instument for tracking load config initializer event from
`config/initializers`
@pftg
Copy link
Contributor Author

pftg commented Nov 16, 2013

Thanks @rafaelfranca and @xaviershay I updated changelog

rafaelfranca added a commit that referenced this pull request Nov 16, 2013
Track initializer's loaded event per file
@rafaelfranca rafaelfranca merged commit 17a0b73 into rails:master Nov 16, 2013
@pftg
Copy link
Contributor Author

pftg commented Nov 16, 2013

Thanks! ❤️

@sheerun
Copy link
Contributor

sheerun commented Dec 5, 2013

awesome

@pftg pftg deleted the track_initializers_loading branch June 9, 2014 00:22
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

Successfully merging this pull request may close these issues.

None yet

6 participants