don't reload in dev mode

Request (unless rails already does this...)

Currently with active_support it reloads the class chain at the
beginning of each incoming HTTP request.

Suggestion: reload the class chain only if a file in a lib path *has
changed*
Thoughts?
-r

The only way to do this in a pure Ruby way is to poll all the files in a thread. I’m not sure if this would be more or less performant. We tried this in Merb and people were annoyed by the constant CPU usage (3-10%) used by polling the entire tree. Something like watchr could make this better.

If you’re interested, a good solution would probably be to make the logic for deciding to reload everything (in Rails, this is “running the to_prepare hooks”) pluggable, such that people could experiment with using watchr et al.

I’ll happily give you a tour of the affected areas if you’re interested.

Yehuda Katz
Architect | Engine Yard
(ph) 718.877.1325

The only way to do this in a pure Ruby way is to poll all the files in a
thread. I'm not sure if this would be more or less performant. We tried this
in Merb and people were annoyed by the constant CPU usage (3-10%) used by
polling the entire tree. Something like watchr could make this better.
If you're interested, a good solution would probably be to make the logic
for deciding to reload everything (in Rails, this is "running the to_prepare
hooks") pluggable, such that people could experiment with using watchr et
al.

That *would* actually be pretty cool :slight_smile:

I'm currently working on making the reload logic pluggable.

Konstantin

I don't think this is necessary. Currently the reloader relies on the usage of require_or_load. Wouldn't it be possible to check at this point whether the file changed or not? That way watching the tree would not be needed.

Also, this is somewhat part of my RSoC project.

Konstantin

Wouldn't that be tricky, even impossible to do right unless you
restrict the current use cases?

Say you have A < B. First request evaluates A and B.

Then you modify B, so you reload B in a second request.

If you do not reload A, its superclass points to an obsolete object.

And that's only an example, there are others of course, and you could
have use cases where the dependency is not discoverable as in

    class C
      @@foo = D.new
    end

If you reload D, you need to reload C. But how do you know that
programmatically?

I think removing everything autoloaded per request is simple and clear.

Wouldn't that be tricky, even impossible to do right unless you
restrict the current use cases?

Say you have A < B. First request evaluates A and B.

Then you modify B, so you reload B in a second request.

If you do not reload A, its superclass points to an obsolete object.

In fact you'll get an error like this:

http://stackoverflow.com/questions/1242559/a-copy-of-applicationcontroller-has-been-removed-from-the-module-tree-but-is-stil

Even leaving aside simple inheritance graphs like that, there are
issues of module inclusion, monkeypatches, 3rd party libraries etc.
It's very very very difficult to get this stuff right. If you simply
watched your app source and avoided reloading if nothing changed, at
least you'd get *some* benefit without finding yourself in the weeds.

inotify, fsevents etc would solve most of the cpu overhead problem.

I don't think this is necessary. Currently the reloader relies on the usage of require_or_load. Wouldn't it be possible to check at this point whether the file changed or not? That way watching the tree would not be needed.

Also, this is somewhat part of my RSoC project.

Glad to hear this. Do you think you'll get around to it or should I
look at it?
-r

FYI. I'm working on external application reloader. It's works as proxy
that checks FS before every request and kills & respawns whole
application if it sees changes.

This approach supports more that just ruby/rails.

The code is here: http://github.com/alk/webdevreloader

Are you addressing the concerns above somehow? (See my email and Koz's.)

Yes, both of them.

Konstantin

>> I'm currently rewriting quite a bit of it. Planning to write a post here or on a blog soon to summarize what I'm changing. When I'm done the reloader will hopefully only kick in on changes and be able to partially reload the class tree.

> Are you addressing the concerns above somehow? (See my email and Koz's.)

Regarding doing multiple File.stat on every request. It works fine. On
one of my past projects which is quite large Rails application, I've
implemented small plugin that checks file changes before every
request. It turned out that fetching stat on every loaded ruby file is
fast. We had no issues with that. And you only need that check before
every request, there's no need to constantly chew CPU. Doing inotify
tricks is fine, but we're good without it.

One of the reasons why this plugin wasn't opensourced is that I had to
adapt it to every major Rails release when we updated our code base,
so it would support only latest. And later my attention turned to
generic approach that's now implemented in webdevreloader. And it
basically does same as my plugin, but instead of purging
ActiveSupport::Dependency loaded classes it kills & respawns whole
application, which allows reloading to support file updates in plugins
and even in Rails itself.

Yes, both of them.

Konstantin

And BTW I don't think partial reloading with all it's fine subtleties
can be made work reliably. In fact initial inspiration for my plugin
was 'dev_mode_performance' plugin which actually tried to do partial
reloading, but it was failing badly on our application. And we had
that plugin for more than 1 year (it's was really large Rails
application) and my experience and experience of my colleagues is that
reloading everything but only when something changes is more than good
enough.

The only reason why I cannot yet recommend just ripping class
reloading out of Rails is few remaining bugs that might be left (and
there's some issue with webrick now) and most importantly lack of
support for windows. BTW, if someone is willing to champion windows
support I would be more than happy. We just need application process
pid and ability to kill it. So it must be easy for anyone with windows
background.

There's a rails-dev-boost (http://github.com/thedarkone/rails-dev-
boost) plugin that patches ActiveSupport::Dependencies to only reload
the changed .rb files. It is not 100% foolproof, but it does some
tricky stuff to work around inheritance/module inclusion edge cases.
It won't handle Xavier's example though. The plugin is also not Rails
3 compatible right now.

A simpler implementation that would first File.mtime the loaded .rb
files for changes (akin to what Jose implemented for the I18n lib)
before calling ActiveSupport::Dependencies.clear is quite
straightforward to implement and should provide snappier development
mode (for example when only templates files are being changed or when
simply clicking around the app).

I'm currently trying to implement the following approach:

As it already is the case, all constants will be removed on every request, however, will be kept with a wrapper in a map. Instead of simply loading files again, AS will check for changes (mtime) as soon as the first const_missing, require_dependency, etc is encountered after the clear. If a file has been changed, constants defined in this file and all 'related' constants will be marked for reload (not instantly reloaded). If a constant marked for reload is auto-loaded the file will be re-evaluated (load), otherwise the constant will simply be restored. This works somewhat like a mark-and-sweep GC, without the sweeping.

What constants are related to each other depends on the reloading strategy. The default strategy is world-reloading (everything will be read from file again). I use a small optimization to avoid looping through all constants in this case: An internal counter is increased and each constant wrapper stores the value that counter had on the last reload. If the stored value is smaller than the current value the class will the treated as if marked for reload.

All other strategies are somewhat experimental:

* Marking all associated constants for reload. Associated constants will be tracked more carefully than in the current implementation. Included mixins and akin will also be added to the associated constants.
* Marking only the constant for reload.
* Marking only the constant for reload and add it to the constant tree again before loading the file. Thus a module/class will monkey-patch itself. This is the approach many other reloader implementations are following. While you are unable to remove methods that way and code will be rerun on the same module, this completely avoids the two most common exceptions (instance of Foo still active and Bar is not missing constant Foo) and also works in cases where old instances are kept somewhere.

Note that a strategy can be chosen on a per constant level. Although if this would be exposed to the user, this should only really matter for plugin developers. For instance, if someone is writing a Redmine plugin, it would only be additional overhead to reload the complete application on plugin changes. The monkey-patching approach would be helpful when embedding for instance a Sinatra application.

At some point after midterm evaluations are through I'm planning to experiment with possible ways to let AS choose the strategy for a class on its own. However, for me this only appears reasonable for the world reloading and the associated constants strategies. One thing I'd imagine is having the option to set strategies on a directory basis, too, and have Rails choose the associated constants strategy for the app folder and the world reloading strategy for the lib folder or something. Although I'm not sure this will be sane. One option I still have in mind is to change the strategy on occurring exceptions: If a class Foo has automatically been set to the associated constants strategy and a 'Foo is still active' exceptions occurs, AS will change the strategy for Foo to world reloading and mark Foo for reload to avoid the exception on the next request.

Restarting the whole ruby process in the background is not easily and efficiently possible in a platform independent way, mainly since Windows does not support fork. Also, it appears to be slower than in-process reloading.

Konstantin

Thinking about your approach. Let me ask you a quick question (written

We have

   class C
     @x = X
   end

First request autoloads X because of an occurrence in a file that is
not c.rb, then it autoloads C because the request needs it somewhere
else.

The AS cache at some point associates the string "X" to the object
stored in X, same for "C".

Now we modify x.rb but not c.rb.

Second request asks for C, but it does not ask for X. Since c.rb has
not been touched, the object associated to the string "C" in the cache
is restored.

What happens with @x?

Yes, this is the main issue with it. @x will of course still point to the old X.

But this issue exist with the current implementation as well:

Currently most Rails developers are not even aware of what is going on when reloading.
And that is ok, as they shouldn't have to be. In fact, talking to some rails novices it seems
that some even believe not reloading but caching the classes is a rails feature (why else
would I have to set cache_classes to true in production mode?). Now the thing is, that
errors keep popping up constantly. I myself had to deal with the reloader issues in every
rails project if been working on lately, especially if it has a large code base and multiple
developers. Mixing unloadable and un-unloadable code causes the same situation.

Mixing require and autoloading, not really knowing when to use require_dependency or
unloadable is just a common reason. But even gems that juggle with classes cannot be used.
What if C is defined in a gem and therefore not marked as unloadable?

Even people being aware of the reloader sometimes appear to have issues with that.

Especially using a normal require in your code somewhere messes up things, as rails is not
touching $LOADED_FEATURES.

I'm aware that reloading the unloadable constants just partially might increase the probability
of such a situation, which is why I would not use it as the default strategy. But there could be an
config option like fast_reloader or something, that would enable it, stating in the comment that
exceptions could increase if dependencies/associations are not set up correctly. Then, if such
an exceptions occurs, it could state in the error message that dependencies should either be set
up correctly or fast_reloader should be disabled and that it is falling back to world reloading, so
the next request should not trigger an exception again. This feature would ease plugin
development in some cases, I think.

I'm not sure whether the association based approach will be useful at all, but it at least fits the
semantics of the current API. What will be very useful is the monkey patching strategy. Writing a
Mixin that is to be used on both an unloadable and an un-unloadable class is not easy currently.
Also, in my experience the monkey-patching approach works very well when writing middleware.
However, as it contradicts established coding styles (like alias_method_chain) and is not 100%
correct (as in not removing methods) it has to always be activated on a per constant level.

Also, an option would be to remove all methods from the old constant as soon as it is reloaded and
create a proxy to the new one. That way @x will always call methods on the correct X. However,
this fails as soon as it comes to instances of X.

Konstantin

No, everything would be wiped unconditionally. And so everything is
afresh later.

It would happen if you store an autoloaded object in something that is
not wiped, like a constant defined in an initializer. If you do in an
initializer

    # Assume MyTwitterClient is autoloaded
    TWITTER_BACKEND = MyTwitterClient

In the second request TWITTER_BACKEND would store a live object (it is
referenced in a valid place as far as Ruby is concerned), that is not
reachable in the AS sense.

But you shouldn't do that. The rule is that autoloaded constants
should play well together.

That's why the current implementation is so robust, because the
concept is easy. You delete everything. Simple. No clever stuff.

Well, the idea is simple, from there to the actual implementation
there are a few hundred lines. Namespaces particularly complicates it
a bit. Dependencies is quite a hack and as Koz said it is hard to get
this stuff right.

Only if everything is unloadable. As soon as require is used somewhere or external code, this no longer is the case.
Sure, this might increase the probability of having old instances around, but it decreases the threat of rerunning code
that is not meant to be run twice.

But you're probably right. So from what I see only world reloading + monkey patching make sense.

No, everything would be wiped unconditionally. And so everything is
afresh later.

Only if everything is unloadable. As soon as require is used somewhere or external code, this no longer is the case.
Sure, this might increase the probability of having old instances around, but it decreases the threat of rerunning code
that is not meant to be run twice.

Yes, yes, I mean all autoloaded stuff is wiped by dependencies. And in
fact it is not a good idea to mix require and autoloading:

    http://advogato.org/person/fxn/diary/526.html

But you're probably right. So from what I see only world reloading + monkey patching make sense.

World reloading (of autoloaded constants) is simple and what we have now.

I think is going to be hard to find an alternative that is worthwhile,
better performance, reasonable implementation/approach, and no
counterexamples.

I am personally fine with the current approach, it is simple and has
not major drawbacks for me[*]. Only autoloads what is used etc. But if
someone finds a better way to manage this it would be awesome!

-- fxn

[*] Though I planned to do a maintenance pass to the implementation
itself for 3.1

Here's a write up of what I did in my fork (also, about code reloading in general): http://rkh.im/2010/08/code-reloading
Feedback welcome.

Konstantin