This project is archived and is in readonly mode.

#1909 ✓resolved
Andrew White

Improve view performance in development and reinstate template reloading in production

Reported by Andrew White | February 8th, 2009 @ 10:35 AM

The attached patch dramatically improves view rendering in development mode when an app becomes complex. It also reinstates template reloading in production mode which was previously removed. It also appears to fix tickets #1818 and #1712.

Basically the idea behind the patch is to make the standard Path an EagerPath and remove EagerPath. Added to this is the template reloading which works in development and production. Newly added and removed templates are then handled on an adhoc basis.

The reason for eager loading in development mode was that the whole view tree was been traversed multiple times by templates_in_path which in my app (20 plugins / 70 controllers) resulted in 12000 lines of fs_usage per request. After this patch the fs_usage is down to 3000 lines and most of that is model reloading, etc.

All the tests currently pass but it needs testing on a few complex applications before I'd be 100% confident of it being error free.

Comments and changes to this ticket

  • Pratik

    Pratik February 8th, 2009 @ 12:43 PM

    • Assigned user set to “josh”
  • josh

    josh February 8th, 2009 @ 05:10 PM

    • Assigned user changed from “josh” to “Yehuda Katz (wycats)”
    • State changed from “new” to “open”
    • Milestone cleared.

    If the functionality is the same, maybe we rename "recompile_modified_templates" back to "cache_template_loading" as that was the original flag.

    I'm +1 on the idea for including it into 2.3. But I'm reassigning to Yehuda to get some comments on the long term consequences.

    Yehuda, is this something that would be pulled into the 3.0 line? Or something you've rewritten. Either why it should still probably be in the 2.3 line. (You can reassign the ticket back to me)

  • David Reese

    David Reese February 8th, 2009 @ 07:06 PM

    • Assigned user changed from “Yehuda Katz (wycats)” to “josh”

    +1 for this for 2.3. Works great on dev/production environments... definite dev performance boost (average render on a quick ab run, down from 179ms to 22ms).

    Rick Olson was going after something similar with rails_dev_mode_performance plugin, before 2.3 started moving stuff around on him.

    Wondering if there's some way the default for production could be recompile_modified_templates = false -- otherwise, if i'm reading it right, it checks the mtime on each template file for each request?

  • David Reese

    David Reese February 8th, 2009 @ 07:08 PM

    Sorry, meant Josh Goebel, not Rick

  • Andrew White

    Andrew White February 8th, 2009 @ 07:09 PM

    • Assigned user changed from “josh” to “Yehuda Katz (wycats)”

    New patch with setting renamed to match previous setting. Also added cache_template_loading = true to default production environment file.

  • Andrew White

    Andrew White February 8th, 2009 @ 07:12 PM

    David, the default is now to not to recompile, i.e. cache_template_loading = true in the default production environment file.

  • Andrew White

    Andrew White February 8th, 2009 @ 07:16 PM

    Also it doesn't check every templates mtime - at least it shouldn't do, if it does let me know. Only templates that are going to be rendered should have the mtime checked.

  • thedarkone

    thedarkone February 8th, 2009 @ 07:28 PM

    Andrew, I have implemented a very similar functionality in rails-dev-boost I assume you've looked at it while making the patch.

    I went through the diff. It is quite well written, but I do have couple of points: 1. You are not properly tracking mtimes and the template (once modified) is thus considered stale forever and is being recompiled every request after that. 2. Once you fix that, you will discover that you have to start tracking methods that are compiled on ActionView::Base::CompiledTemplates and manually undef all of them on stale templates (the plugin does that). 3. I would also suggest you introduce something similar to @disk_cache (as it is in the plugin). I am pretty sure you are running Dir.glob on pretty much every find_template request. 4. It probably is not thread safe (neither is my implementation in the plugin), is something that is not thread safe going to be accepted into core?

    On the other hand Rails 2.3 performance in the development mode is appalling and developing a 70 controller app becomes really painful. I also think we should be paying a little bit more attention to how slow the development mode is, after all every Rails developer in the world uses dev. mode to build his apps. Isn't not-waisting developer's time one of the core ideas behind Rails?

    I'm +1 on the patch (after some fixes of course).

    -thedarkone

    PS: Andrew, have you tried running your app with rails-dev-boost?

  • thedarkone

    thedarkone February 8th, 2009 @ 07:36 PM

    Damn the formatting :(.

    And in case anyone cares, rails-dev-boost is an improved (Rails 2.2 & 2.3 compatible) alternative to the Josh Goebel's rails_dev_mode_performance, so if anyone is having really bad time with development mode, give it a try :).

  • Andrew White

    Andrew White February 9th, 2009 @ 07:50 AM

    Attached is a new patch which addresses some of the issues mentioned.

    I've handled the threadsafe issue for the moment by forcing to cached templates only. The mtime is now tracked and it now works properly when partials have been changed.

    I'm not removing the methods because I'm sure I read somewhere that it leaks memory, plus I'm assuming that it wouldn't be threadsafe. The Dir.glob in find_template is only for finding possible templates within the base_path so only ever matches a few files and is a one time hit when a newly added template is requested.

    I did look at rails-dev-boost and did try running it with my app but there were some conflicts with the dependencies modifications. I did have a look before writing this patch but didn't use it anyway since I wanted to touch the minimum of code in this patch so that it would stand a good chance of getting into 2.3, so I'm sure it's not optimal.

    I would hope for 3.0 that as part of the public API the template picking and caching would be pluggable so I fully expect this to be chucked at some point.

  • Bodaniel Jeanes

    Bodaniel Jeanes February 9th, 2009 @ 12:45 PM

    +1 for including this in 2.3 -- and bonus points for fixing 2 other tickets I am watching closely!

  • Chris Heald

    Chris Heald February 9th, 2009 @ 05:00 PM

    This patch completely resolved #1922 for me. +1 for including in 2.3.

  • Chris Heald

    Chris Heald February 9th, 2009 @ 05:04 PM

    Joshua, for what it's worth, if 2.3 goes gold without a fix like this, it's going to be neigh-unusable for me. Adding an additional 2 seconds per page reload is going to destroy my development productivity, and I'd much rather develop against a faster 2.2.2 than a slower, more featureful 2.3. The slower view performance is a definite regression from 2.2.2.

  • josh

    josh February 9th, 2009 @ 06:06 PM

    @Chris woah, don't worry, its going get in. Just waiting on Yehuda's comments.

  • Chris Heald

    Chris Heald February 9th, 2009 @ 06:09 PM

    Woot. Very glad to hear that. :)

  • josh

    josh February 9th, 2009 @ 08:14 PM

    • Assigned user changed from “Yehuda Katz (wycats)” to “josh”
  • Repository

    Repository February 9th, 2009 @ 08:20 PM

    • State changed from “open” to “resolved”

    (from [893e9eb99504705419ad6edac14d00e71cef5f12]) Improve view rendering performance in development mode and reinstate template recompiling in production [#1909 state:resolved]

    Signed-off-by: Joshua Peek josh@joshpeek.com http://github.com/rails/rails/co...

  • josh

    josh February 9th, 2009 @ 09:44 PM

    • State changed from “resolved” to “open”

    Railties tests failed, I thought the view paths stuff needed to be restored (http://github.com/rails/rails/co..., but there is still an issue?

  • Stephan Kaag

    Stephan Kaag February 9th, 2009 @ 09:47 PM

    Yes there is.

    My engine is running fine without cache_classes turned on but in production environment i get an error during server boot.

    /vendor/rails/railties/lib/initializer.rb:374:in load_view_paths': undefined local variable or methodview_path' for #Rails::Initializer:0x1a59334 (NameError)

  • thedarkone

    thedarkone February 9th, 2009 @ 11:18 PM

    Oh shit, it already got commited? The patch doesn't cover some non obvious edge cases that will painful to debug.

    Here what I was going to write, before refreshing the page:

    Andrew, I'd suggest you rewrite the mtime checking thing without using memoize at all, it does have some overhead and you aren't really using it to memoize anything, but that's just my opinion.

    Now onto the important stuff: I don't think remove_method leaks memory on methods defined through evaled def abc string, plus as you said, you don't care about thread safety of reloading templates. However all that is irrelevant, you still need to track the compiled methods and recompile/remove them manually, I don't think there is a simple way round it :).

    
      index.html.erb:
      <%= render 'test' %>
      <%= render 'test', :a => 'a' %>
      
      _test.html.erb:
      a
    

    Change _test.html.erb and see what you get back.

    As for Dir.glob - unless all your templates have locale extensions your implementation does invoke Dir.glob in every find_template call.

    I'm sorry if I sound like a major PITA here, but the stuff the plugin does and you patch doesn't is there for a reason.

  • thedarkone

    thedarkone February 10th, 2009 @ 12:59 AM

    I attempted to port my patch from the plugin. I'm 95% certain that it works (the core is the same, I just had to wire it up with the framework differently), but it does have shitload of tests failing. I'm not really familiar with actionpack test scaffolding and therefore quite lost. If anyone is up for the task of fixing them... help is appreciated :).

    Add-reloadable-templates-HEAD2.patch should be against pre-patch.

    Add-reloadable-templates-HEAD.patch should be against current master.

    Since I do have some expertise at implementing reloadable templates, I'll try to go through any other patch that gets posted here :).

  • thedarkone
  • thedarkone

    thedarkone February 10th, 2009 @ 02:09 AM

    Wow, I do suck at making diffs :(.

    Attaching new patch against current master/HEAD.

  • josh

    josh February 10th, 2009 @ 05:29 AM

    I don't know about that "ActionView::ReloadableTemplate.register_new_request_callback!(ActionMailer::Base)" stuff.

    We don't want that kind of initializer code in the dispatcher nor do we want to be referencing ActionMailer when someone could decide to not include it. AC should not reference AM.

  • Andrew White

    Andrew White February 10th, 2009 @ 06:44 AM

    Okay, here's a patch that addresses the edge case of the same template being called with different local assigns. I've also tweaked it slightly so that when perform_caching is disabled then templates are always reloaded from the disk. This approximates to the old behaviour.

    Rather than remove methods I'm tracking the timestamp of when the source was compiled and the timestamp of when a method is defined. If the timestamp of compilation is later than the definition then it gets redefined. Removing methods was causing some of the tests to fail.

    I've removed the load_framework_views code again and tweaked the initializer test to call initialize_framework_views instead. There were the two methods before because we had Path and EagerPath but now they've been merged we don't need load_view_paths.

    I've also testing with some of my own engine code and it seems to work fine.

    Both ActionPack and Railties tests pass.

  • Gaspard Bucher

    Gaspard Bucher February 10th, 2009 @ 12:03 PM

    Current rails edge is broken due to a typo in initializer.rb.

    This patch fixes this.

  • Andrew White

    Andrew White February 10th, 2009 @ 12:13 PM

    Gaspard, just a note that the patch that I posted this morning removes those lines anyway. Josh put them back in because of an Initializer test which called load_view_paths.

    The initial purpose of load_view_paths was to eager load templates but since EagerPath has been folded into Path then there's no need for it as initialize_framework_views does the job now and doesn't stamp all over engine view paths.

  • LacKac

    LacKac February 10th, 2009 @ 03:03 PM

    Haven't looked at all the comments (i'm in a hurry right now) but commented on the commit an issue I'm having when using external template handlers: http://github.com/rails/rails/co...

  • Andrew White
  • LacKac

    LacKac February 10th, 2009 @ 04:44 PM

    Andrew, yes, it seems to work. Thanks.

  • josh

    josh February 10th, 2009 @ 06:06 PM

    • State changed from “open” to “resolved”

    Applied

  • LacKac

    LacKac February 10th, 2009 @ 08:58 PM

    I still have problems with this. Now it seems that it is overcaching in development (or maybe I'm not getting how this should work). First I thought it's JRuby or Haml, but I could reproduce in a fresh bare rails app run on MRI.

    
    $ /path/to/edge/rails/bin/rails test
    $ cd test
    $ ln -s /path/to/edge/rails vendor/rails
    $ script/generate scaffold Post title:string content:text
    $ rake db:migrate
    $ script/server
    

    Now visit http://localhost:3000/posts/new and then change the title from "New post" to "Brand new post" in app/views/posts/new.html.erb. Reload in the browser and you will still see the original title. Only by restarting the server did I get the new text.

  • thedarkone

    thedarkone February 10th, 2009 @ 11:27 PM

    New version of my patch 0001-Add-reloadable-templates.patch: kicked out objectionable stuff from Dispatcher, reinstated _pick_partial_template cache that was removed by Andrews' patch and fixed failing tests.

    Is there a chance that my version of the patch could be accepted? It does some stuff differently, mainly by creating ReloadableTemplate and ReloadablePath classes as I think that makes it much more clear what each part of the code does. I am also pretty certain it is faster and I don't want to be patching this stuff again in rails-dev-boost once 2.3 is out.

  • thedarkone

    thedarkone February 10th, 2009 @ 11:34 PM

    Changed whitespacing in ReloadableTemplate to be consistent with Rails' style.

  • josh

    josh February 10th, 2009 @ 11:46 PM

    Please comment on thedarkone's patch. Andrew what do you think?

  • LacKac

    LacKac February 11th, 2009 @ 01:40 AM

    Josh, this last patch seems to solve my problems. Haml templates work and the views are not overcached.

  • René Rask

    René Rask February 11th, 2009 @ 02:22 AM

    +1 for the darkone patch: 0001-Add-reloadable-templates.patch

    Current edge requires a server restart to recompile the views in development mode (but they recompile when changed in production mode). The darkone patch fixes this problem.

  • josh

    josh February 11th, 2009 @ 02:43 AM

    • State changed from “resolved” to “open”
  • josh
  • Andrew White

    Andrew White February 11th, 2009 @ 07:00 AM

    Here's a patch to fix the reloading when perform_caching is false. Not sure why it wasn't picked up by the test though.

    I'm going over the latest patch from thedarkone will let you have my comments in a bit

  • Andrew White

    Andrew White February 11th, 2009 @ 07:32 AM

    This patch fixes both the reloading and threadsafe issues

  • Andrew White

    Andrew White February 11th, 2009 @ 10:35 AM

    I've had a look over thedarkone's patch and I do like the way it's implemented and the main reason I didn't do something similar was I wanted to try touch as little as possible so that it would be accepted for 2.3.

    However, at the moment the way the testing is done is that virtually all of the tests are on EagerPath view paths rather than ReloadablePath view paths - only test_template_changes_are_reflected_without_cached_template_loading is actually tested using ReloadablePath. This is because the patch sets cache_template_loading to true in abstract_unit.

    Since my changes reduce the Path implementations down to just a single one I feel that my changes have undergone more testing so before I can endorse thedarkone's patch I'd like to see him fix the attached test failures when running with cached_template_loading = false.

  • thedarkone

    thedarkone February 11th, 2009 @ 05:58 PM

    Thanks for the feedback Andrew :).

    First of all, I am pretty sure neither of our patches are thread safe, both of them are also leaking memory (because of how ActionView works) and should be used in production only if reloadable templates are an absolutely necessity.

    I changed tests to be run with EagerPaths, because that is how your tests run pretty much. To see what I mean, put something like this in abstract_unit.rb to truly simulate development mode:

    
    ActionView::Template::Path.class_eval do
      def load!
        @paths = {}
      end
    end
    

    And watch things blow up.

    Majority of tests that were failing with my patch were of this kind: assert_equal ActionView::Template::EagerPath, view_paths.first.class

    There was also some funky stuff with relative paths like this ./.././././abc not being properly handled, I've fixed that.

    I've also droped all the Dispatcher.to_prepare callback magic as it wouldn't notice prepend_view_paths calls on a class level.

    I've kept cache_template_view set to true in abstract_unit.rb for faster running tests, but all tests are now passing for ReloadableTemplates as well.

    0002-Add-reloadable-templates.patch is the new patch.

  • thedarkone

    thedarkone February 11th, 2009 @ 06:11 PM

    One more comment, right now the default value for cached_template_loading is false, I would argue that the default value should be true and that cache_classes = false would always flip it to false.

    cached_template_loading = false by default: pros: template caching is fully configurable in enviroment files cons: existing apps would have to manually set cached_template_loading = true in production.rb on upgrade, I'm sure this will cause some problems :)

    cached_template_loading = true by default: pros: upgrading to 2.3 requires no changes to production.rb cons: cache_classes = false will always flip it to false and you won't be able to have development mode with reloading classes, but cached templates (that's a retarded combination anyway)

  • thedarkone

    thedarkone February 11th, 2009 @ 06:13 PM

    Screwed by Lighthouse formatting yet again :(.

  • thedarkone

    thedarkone February 11th, 2009 @ 08:46 PM

    0003-Add-reloadable-templates.patch: restored dual (reloadable vs. cached) tests in test/template/render_test.rb that were removed by Andrew's patch.

  • josh

    josh February 12th, 2009 @ 06:20 PM

    These kinds of things are such a pain since there are so many edge cases and they all have to be tested manually.

    1. Make sure templates reload in development mode
    2. Make sure templates don't reload in production mode unless cache_template_loading is set to true
    3. Test performance and memory leakage
    4. Threadsafety
  • josh

    josh February 12th, 2009 @ 06:21 PM

    0003-Add-reloadable-templates.patch

    railties fail

    1) Failure: test_engine_controllers_should_have_their_view_path_set_when_loaded(TestPluginLoader)

    [test/plugin_loader_test.rb:133:in `test_engine_controllers_should_have_their_view_path_set_when_loaded'
     mocha (0.9.5) lib/mocha/test_case_adapter.rb:69:in `__send__'
     mocha (0.9.5) lib/mocha/test_case_adapter.rb:69:in `run']:
    
    

    <["./test/fixtures/plugins/engines/engine/app/views"]> expected but was <["test/fixtures/plugins/engines/engine/app/views"]>.

  • thedarkone2

    thedarkone2 February 12th, 2009 @ 06:45 PM

    I've talked with Andrew and his only remaining objection to my patch is that it was rescueing from any errors while removing compiled methods like this:

    
    def undef_my_compiled_methods!
     @compiled_methods.each {|comp_method|
    ActionView::Base::CompiledTemplates.send(:remove_method, comp_method)
    rescue nil}
     @compiled_methods.clear
    end
    

    That was necessary because compiled_templates_test.rb was kinda blunt about removing methods from CompiledTemplates. I rewrote the test and removed the rescue clause.

    There is however some potential for breakage if a plugin tampers with CompiledTemplates without notifying ReloadableTemplate about its intentions. Andrew suggests adding a tracking mechanism to CompiledTemplates that would try to handle some cases. I'm arguing that a plugin author should handle any edge cases himself.

  • thedarkone2

    thedarkone2 February 12th, 2009 @ 06:59 PM

    I'm posting from a new account, because Lighthouse is being a bitch and always errors out on me (since yesterday), if I try to access the ticket as thedarkone.

    Josh, there should not be any problems with the plugin not working in dev or prod mode. It also should be very fast.

    The memory leak I was talking about is about 2 issues:

    1. leaks through symbols, due to methods compiled on CompiledTemplates (un-fixable due to how template compilation works),

    2. leaks through deleted templates that are never accessed by a new/changed templates, i.e. if a controller is deleted and its templates are never accessed again, they still will remain in memory because everything is written to be super-lazy.

    Reloadable templates are not thread safe and ActionController::Base.allow_concurency = true always disables template reloading.

  • josh

    josh February 12th, 2009 @ 07:10 PM

    0004-Add-reloadable-templates.patch applied.

    Please test.

  • josh

    josh February 12th, 2009 @ 08:02 PM

    • State changed from “open” to “resolved”

    Just did a quick test. Looks solid. Good work guys.

    If anymore issues pop up I'll be sure to ping you ;)

  • AQ

    AQ February 17th, 2009 @ 08:45 PM

    Maybe this is the wrong place for a newbie question but I tried apply this patch to edge rails in /vendor to fix the missing template error (#1712) but I get lots of errors like:

    error: actionmailer/test/abstract_unit.rb: does not exist in index error: actionmailer/test/mail_service_test.rb: does not exist in index error: actionpack/lib/action_controller/rescue.rb: does not exist in index

    after running "git am < 0004-Add-reloadable-templates.patch"

  • thedarkone

    thedarkone February 18th, 2009 @ 02:04 AM

    Hey AQ, I'm not familiar with #1712, but 0004-Add-reloadable-templates.patch has already been applied and is now a part of edge Rails: http://github.com/rails/rails/co...

  • nachocab (at gmail)

    nachocab (at gmail) February 18th, 2009 @ 06:41 AM

    Hey everyone, I'm still getting the MissingTemplate error using latest version of edge rails and latest version of Haml. Do I need to configure something in environment.rb? Where can I find the line of code that renders the template through Haml? Thanks

  • Andrew White

    Andrew White February 18th, 2009 @ 04:33 PM

    Just found a slight edge case.

    If you alter the view paths in development mode using a string then when the class gets reloaded and the view paths are reprocessed blowing away the existing templates. The templates are recreated but since they don't know about the compiled methods then they aren't deleted and therefore aren't recompiled.

    In production the classes are cached so the view paths are reprocessed so any template modifications are reloaded automatically.

  • thedarkone

    thedarkone February 18th, 2009 @ 07:53 PM

    Ouch, that's one ugly edge case :(.

    0001-Fix-an-edge-case-where-we-might-encounter-some-rando.patch gives up on manually tracking compiled methods and just greps public_instance_methods for any suspects.

  • josh

    josh February 18th, 2009 @ 08:47 PM

    Can you please open a new ticket for related issues. I'm getting lost in this long thread ;)

  • Andrew White
  • Sven Fuchs

    Sven Fuchs February 20th, 2009 @ 05:59 PM

    Don't you guys think you're introducing a bit too much new stuff here? 2.3 is supposed to be in a release candidate state.

    We're trying to port an existing app that extends ActionView to 2.3 so we can start using 2.3 as soon as it hits a stable release. But ActionView moves so fast that this seems virtually impossible.

    An api freeze for ActionView would help a lot with that.

    Please don't get me wrong. I don't want to rain on anybody's work or party. And I love you guys for all the awesome work you're putting in here. :D

  • josh

    josh February 20th, 2009 @ 06:16 PM

    Sven, yeah it hasn't turned out so well. But most people considered the pervious development mode slowness and unacceptable bug.

    This stuff + rack changes is really making this a rough RC process. Lucky for me, I'm in charge of both these areas :)

  • Sven Fuchs

    Sven Fuchs February 20th, 2009 @ 06:39 PM

    Yeah, and you're doing some damn awesome work in both of them :)

  • thedarkone

    thedarkone February 20th, 2009 @ 07:10 PM

    Sven, is it related to your adva_cms and how it allows for some templates to be dynamically changed in production?

    The ReloadableTemplates patch, as it is right now, really goes out of its way to avoid touching anything related to how fully cached templates work in production. Almost nothing should have been changed between RC1 and master about how ActionView's default production environment works.

    I didn't dig that deep, but couldn't you be using core's ReloadableTemplates in production by simply setting config.action_view.cache_template_loading = false?

  • Sven Fuchs

    Sven Fuchs February 20th, 2009 @ 07:42 PM

    Hey thedarkone,

    thanks for the suggestions! Yeah, this is about our theme support in adva_cms.

    I'm checking out your code right now. Your implementation actually looks better than what we've previously been trying, better encapsulated etc. So, of course this is pretty cool to have in core.

    Of course I'd rather not want to turn off cache_template_loading for the whole app. We add a theme load path on a per controller instance/request basis. I'm thinking of just adding a ReloadablePath here. Reading your code I figure that even might work without any further modification. Unfortunately I can't really test things right now with existing tests because Webrat seems to be broken on edge, too. :/

    Anyway. Thanks again!

  • Andrew White

    Andrew White February 21st, 2009 @ 04:45 AM

    Sven, I thought you might have problems - one of the things we left out was overriding remove_method in CompiledTemplates to track other plugins, etc. that tried to remove compiled template methods. I found your app via a github search but yours was the only one so we left it out.

    I've been using this code for a few days now on my ecommerce app and its been stable so hopefully we've eliminated most of the regressions.

    My app also uses per-request view paths and the way I handle it is call process_view_paths on each themes view path on startup and store them in a hash keyed by theme name. That way I don't need to directly deal with ReloadablePath or EagerPath - the cache_template_loading config will deal with it. The only issue is that I need to restart the app to see any new themes so if you want to support something like uploading of new themes you'd want to process the view path after uploading and unpacking the theme.

  • Andrew White

    Andrew White February 21st, 2009 @ 04:56 AM

    Also the performance enhancement in this patch is huge - previously it was taking about three seconds to render a page in my app as the views paths were constantly walked and now it's down to half a second. Even better is that with template reloading restored to production mode I can create and edit themes for my app in production mode so the productivity boost is even greater.

    It's been a bit of rocky path over the past couple weeks but hopefully you'll agree it's been worth it.

  • Repository

    Repository April 13th, 2009 @ 11:58 PM

    (from [906aebceedb95d8caa6db6314bc90f605bdfaf2b]) Bring abstract_controller up to date with rails/master

    Resolved all the conflicts since 2.3.0 -> HEAD. Following is a list of commits that could not be applied cleanly or are obviated with the abstract_controller refactor. They all need to be revisited to ensure that fixes made in 2.3 do not reappear in 3.0:

    2259ecf368e6a6715966f69216e3ee86bf1a82a7 AR not available * This will be reimplemented with ActionORM or equivalent

    06182ea02e92afad579998aa80144588e8865ac3 implicitly rendering a js response should not use the default layout [#1844 state:resolved] * This will be handled generically

    893e9eb99504705419ad6edac14d00e71cef5f12 Improve view rendering performance in development mode and reinstate template recompiling in production [#1909 state:resolved] * We will need to reimplement rails-dev-boost on top of the refactor;

    the changes here are very implementation specific and cannot be
    cleanly applied. The following commits are implicated:
    
      199e750d46c04970b5e7684998d09405648ecbd4
      3942cb406e1d5db0ac00e03153809cc8dc4cc4db
      f8ea9f85d4f1e3e6f3b5d895bef6b013aa4b0690
      e3b166aab37ddc2fbab030b146eb61713b91bf55
      ae9f258e03c9fd5088da12c1c6cd216cc89a01f7
      44423126c6f6133a1d9cf1d0832b527e8711d40f
    
    

    0cb020b4d6d838025859bd60fb8151c8e21b8e84 workaround for picking layouts based on wrong view_paths [#1974 state:resolved] * The specifics of this commit no longer apply. Since it is a two-line

    commit, we will reimplement this change.
    
    

    8c5cc66a831aadb159f3daaffa4208064c30af0e make action_controller/layouts pick templates from the current instance's view_paths instead of the class view_paths [#1974 state:resolved] * This does not apply at all. It should be trivial to apply the feature

    to the reimplemented ActionController::Base.
    
    

    87e8b162463f13bd50d27398f020769460a770e3 fix HTML fallback for explicit templates [#2052 state:resolved] * There were a number of patches related to this that simply compounded

    each other. Basically none of them apply cleanly, and the underlying
    issue needs to be revisited. After discussing the underlying problem
    with Koz, we will defer these fixes for further discussion.
    
    

    http://github.com/rails/rails/co...

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Referenced by

Pages