141 3 true 2008-11-12T11:12:05+00:00 813 2009-01-31T00:00:00+00:00 1359 add-optional-format-argument-to-named-routes 18 8994 committed actionpack options patch resources 2008-11-26T09:57:48+00:00 17393 Repository Tom Stuart Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/1359 The discussion in #1215 has investigated ways to reduce the number of memory-hungry routes generated by `map.resources`. Several people have suggested that we could abandon the `formatted_` named routes altogether and replace them with a `:format` parameter to the regular named routes. This seems like a more natural implementation and will avoid cluttering the route set for people who rarely or never use these routes. Related links oourtesy of aaronbatalion: * http://blog.hungrymachine.com/2008/11/11/how-to-save-100m-of-ram-per-mongrel-part-2 * http://gist.github.com/23712 * http://blog.hungrymachine.com/2008/11/8/how-to-save-100m-of-ram-per-mongrel Can we get a patch together? The discussion in #1215 has investigated ways to reduce the number of memory-hungry routes generated by `map.resources`. Several people have suggested that we could abandon the `formatted_` named routes altogether and replace them with a `:format` parameter to the regular named routes. This seems like a more natural implementation and will avoid cluttering the route set for people who rarely or never use these routes. Related links oourtesy of aaronbatalion: * http://blog.hungrymachine.com/2008/11/11/how-to-save-100m-of-ram-per-mongrel-part-2 * http://gist.github.com/23712 * http://blog.hungrymachine.com/2008/11/8/how-to-save-100m-of-ram-per-mongrel Can we get a patch together? <div><p>The discussion in <a href="/projects/8994/tickets/1215" title="Ticket #1215">#1215</a> has investigated ways to reduce the number of memory-hungry routes generated by <code>map.resources</code>.</p> <p>Several people have suggested that we could abandon the <code>formatted_</code> named routes altogether and replace them with a <code>:format</code> parameter to the regular named routes. This seems like a more natural implementation and will avoid cluttering the route set for people who rarely or never use these routes.</p> <p>Related links oourtesy of aaronbatalion:</p> <ul> <li><a href="http://blog.hungrymachine.com/2008/11/11/how-to-save-100m-of-ram-per-mongrel-part-2"> http://blog.hungrymachine.com/20...</a></li> <li><a href="http://gist.github.com/23712">http://gist.github.com/23712</a></li> <li><a href="http://blog.hungrymachine.com/2008/11/8/how-to-save-100m-of-ram-per-mongrel"> http://blog.hungrymachine.com/20...</a></li> </ul> <p>Can we get a patch together?</p></div> 141 0 The discussion in #1215 has investigated ways to reduce the number of memory-hungry routes generated by `map.resources`. Several people have suggested that we could abandon the `formatted_` named routes altogether and replace them with a `:format` parameter to the regular named routes. This seems like a more natural implementation and will avoid cluttering the route set for people who rarely or never use these routes. Related links oourtesy of aaronbatalion: * http://blog.hungrymachine.com/2008/11/11/how-to-save-100m-of-ram-per-mongrel-part-2 * http://gist.github.com/23712 * http://blog.hungrymachine.com/2008/11/8/how-to-save-100m-of-ram-per-mongrel Can we get a patch together? <div><p>The discussion in <a href="/projects/8994/tickets/1215" title="Ticket #1215">#1215</a> has investigated ways to reduce the number of memory-hungry routes generated by <code>map.resources</code>.</p> <p>Several people have suggested that we could abandon the <code>formatted_</code> named routes altogether and replace them with a <code>:format</code> parameter to the regular named routes. This seems like a more natural implementation and will avoid cluttering the route set for people who rarely or never use these routes.</p> <p>Related links oourtesy of aaronbatalion:</p> <ul> <li><a href="http://blog.hungrymachine.com/2008/11/11/how-to-save-100m-of-ram-per-mongrel-part-2"> http://blog.hungrymachine.com/20...</a></li> <li><a href="http://gist.github.com/23712">http://gist.github.com/23712</a></li> <li><a href="http://blog.hungrymachine.com/2008/11/8/how-to-save-100m-of-ram-per-mongrel"> http://blog.hungrymachine.com/20...</a></li> </ul> <p>Can we get a patch together?</p></div> false 2008-11-12T11:12:06+00:00 813 --- {} 9903 1359 add-optional-format-argument-to-named-routes 0 8994 new actionpack options resources 2008-11-12T11:12:10+00:00 813 Tom Stuart Tom Stuart Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/1359 2.x 141 0 I like this. Seems that http://gist.github.com/23712 has the pointers for making this so. Just needs to be wrapped up in a real patch with tests. <div><p>I like this. Seems that <a href="http://gist.github.com/23712">http://gist.github.com/23712</a> has the pointers for making this so. Just needs to be wrapped up in a real patch with tests.</p></div> false 2008-11-12T11:55:07+00:00 813 --- {} 9903 1359 add-optional-format-argument-to-named-routes 0 8994 new actionpack options resources 2008-11-12T11:55:13+00:00 83 DHH Tom Stuart Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/1359 2.x 141 0 At the same time it would be great to continue jeremy's work to cut down on the memory usage from the generated recognition code. <div><p>At the same time it would be great to continue jeremy's work to cut down on the memory usage from the generated recognition code.</p></div> false 2008-11-12T12:01:35+00:00 813 --- {} 9903 1359 add-optional-format-argument-to-named-routes 0 8994 new actionpack options resources 2008-11-12T12:01:38+00:00 141 Michael Koziarski Tom Stuart Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/1359 2.x 141 0 I was working on a real patch with tests already. Will update soon. There are a couple known bugs in the above gist. <div><p>I was working on a real patch with tests already. Will update soon. There are a couple known bugs in the above gist.</p></div> false 2008-11-12T12:08:49+00:00 813 --- {} 9903 1359 add-optional-format-argument-to-named-routes 0 8994 new actionpack options resources 2008-11-12T12:08:51+00:00 37059 aaronbatalion Tom Stuart Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/1359 2.x 141 0 Attached is the patch for optional .:format in routes, which decreases the number of routes by 50%, saving up to 100M of RAM on larger rails apps. Notes: Found one side effect when PageCachingTest. In RouteSet#routes_for_controller_and_action_and_keys, routes are sorted by significant keys, subtracting the keys that are passed in. Therefore, are UrlRewriter.rewrite(:controller => "foo", :format => nil) and UrlRewriter.rewrite(:controller => "foo") are not the same. I've added a test to UrlRewriterTest, and modified the RouteSet#routes_for_controller_and_action_and_keys to remove pairs with nil values, and left the PageCachingTest alone. <div><p>Attached is the patch for optional .:format in routes, which decreases the number of routes by 50%, saving up to 100M of RAM on larger rails apps.</p> <p>Notes: Found one side effect when PageCachingTest.</p> <p>In RouteSet#routes_for_controller_and_action_and_keys, routes are sorted by significant keys, subtracting the keys that are passed in. Therefore, are UrlRewriter.rewrite(:controller =&gt; "foo", :format =&gt; nil) and UrlRewriter.rewrite(:controller =&gt; "foo") are not the same.</p> <p>I've added a test to UrlRewriterTest, and modified the RouteSet#routes_for_controller_and_action_and_keys to remove pairs with nil values, and left the PageCachingTest alone.</p></div> false 2008-11-13T02:08:07+00:00 813 --- :tag: actionpack options resources 9903 1359 add-optional-format-argument-to-named-routes 0 8994 new actionpack options patch resources 2008-11-13T02:08:09+00:00 37059 aaronbatalion Tom Stuart Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/1359 2.x 141 1 Aaron, Attached is an updated diff, compatible with Tom's changes from http://github.com/rails/rails/commit/4c0921024471c0463d67f8b8fb6a115a94d343aa All tests passing and just piped it through the test suite of a large app with a huge number of nested routes. All seems well.Great track of thought with this ! - Lourens <div><p>Aaron,</p> <p>Attached is an updated diff, compatible with Tom's changes from <a href="http://github.com/rails/rails/commit/4c0921024471c0463d67f8b8fb6a115a94d343aa"> http://github.com/rails/rails/co...</a></p> <p>All tests passing and just piped it through the test suite of a large app with a huge number of nested routes.</p> <p>All seems well.Great track of thought with this !</p> <ul> <li>Lourens</li> </ul></div> false 2008-11-13T17:42:21+00:00 813 --- {} 9903 1359 add-optional-format-argument-to-named-routes 0 8994 new actionpack options patch resources 2008-11-13T17:42:24+00:00 25849 Lourens Naudé Tom Stuart Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/1359 2.x 141 2 Really nice work so far guys, if we do this we need to think about a few things: # generation optimisations They assume that all segments are mandatory. This changes that and will cause them to fail to kick in even when they should. # Deprecating the formatted_... routes nicely the formatted routes need to warn you, and continue to work with positional arguments formatted_person_url(1, :xml) <div><p>Really nice work so far guys, if we do this we need to think about a few things:</p> <h1>generation optimisations</h1> <p>They assume that all segments are mandatory. This changes that and will cause them to fail to kick in even when they should.</p> <h1>Deprecating the formatted_... routes nicely</h1> <p>the formatted routes need to warn you, and continue to work with positional arguments</p> <p>formatted_person_url(1, :xml)</p></div> false 2008-11-14T10:35:21+00:00 813 --- {} 9903 1359 add-optional-format-argument-to-named-routes 0 8994 new actionpack options patch resources 2008-11-14T10:35:26+00:00 141 Michael Koziarski Tom Stuart Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/1359 2.x 141 2 I'd really like to see this make it into 2.3. The formatted_ stuff was a hack anyway. Would be great to get rid of it. Anyone working on this have some comments for koz's concerns? <div><p>I'd really like to see this make it into 2.3. The formatted_ stuff was a hack anyway. Would be great to get rid of it. Anyone working on this have some comments for koz's concerns?</p></div> false 2008-11-23T13:17:31+00:00 813 --- :milestone: 9903 1359 add-optional-format-argument-to-named-routes 0 8994 new actionpack options patch resources 2008-11-23T13:17:33+00:00 83 DHH Tom Stuart Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/1359 141 2 Attached is a new patch that gets rid of :format's, and answers Koz's concerns. 1) Included a deprecation warn for each use of formatted_url* methods. If someone would like to suggest the proper warning text, I'm sure it can be improved. 2) Fixed the generation optimisations implementation to still work for the previously route.optimise?-able routes. 3) Started to rip out all direct calls to formatted_*, but that might need another pass to completely deprecate it. <div><p>Attached is a new patch that gets rid of :format's, and answers Koz's concerns.</p> <p>1) Included a deprecation warn for each use of formatted_url* methods. If someone would like to suggest the proper warning text, I'm sure it can be improved.</p> <p>2) Fixed the generation optimisations implementation to still work for the previously route.optimise?-able routes.</p> <p>3) Started to rip out all direct calls to formatted_*, but that might need another pass to completely deprecate it.</p></div> false 2008-11-24T07:41:16+00:00 813 --- {} 1359 add-optional-format-argument-to-named-routes 0 8994 new actionpack options patch resources 2008-11-24T07:41:21+00:00 37059 aaronbatalion Tom Stuart Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/1359 141 2 correct patch attached. <div><p>correct patch attached.</p></div> false 2008-11-24T07:43:15+00:00 813 --- {} 1359 add-optional-format-argument-to-named-routes 0 8994 new actionpack options patch resources 2008-11-24T07:43:17+00:00 37059 aaronbatalion Tom Stuart Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/1359 141 3 (from [fef6c32afe2276dffa0347e25808a86e7a101af1]) Added optimal formatted routes to rails, deprecating the formatted_* methods, and reducing routes creation by 50% [#1359 state:committed] Signed-off-by: David Heinemeier Hansson <david@loudthinking.com> http://github.com/rails/rails/commit/fef6c32afe2276dffa0347e25808a86e7a101af1 <div><p>(from [fef6c32afe2276dffa0347e25808a86e7a101af1]) Added optimal formatted routes to rails, deprecating the formatted_* methods, and reducing routes creation by 50% [<a href="/projects/8994/tickets/1359" title="Ticket #1359">#1359</a> state:committed]</p> <p>Signed-off-by: David Heinemeier Hansson <a href="mailto:david@loudthinking.com">david@loudthinking.com</a> <a href="http://github.com/rails/rails/commit/fef6c32afe2276dffa0347e25808a86e7a101af1"> http://github.com/rails/rails/co...</a></p></div> true 2008-11-26T09:57:47+00:00 813 --- :state: new 1359 add-optional-format-argument-to-named-routes 0 8994 committed actionpack options patch resources 2008-11-26T09:57:48+00:00 17393 Repository Tom Stuart Michael Koziarski http://rails.lighthouseapp.com/projects/8994/tickets/1359 bbf598c8579e7a809c5beb1258d2e93b9fb3ef16 text/plain 2008-11-13T02:08:07+00:00 optional_format_token_in_resource_routes.diff 61756 5881 37059 http://rails.lighthouseapp.com/attachments/61756/optional_format_token_in_resource_routes.diff b6da3c0d0ba03af7f0f4cff792710c7f0eda7694 text/plain 2008-11-13T17:42:21+00:00 optional_format_token_in_resource_routes.diff 61949 5926 25849 http://rails.lighthouseapp.com/attachments/61949/optional_format_token_in_resource_routes.diff 2a7be035c45dd0aabe502875c2d23080083aa13f text/plain 2008-11-24T07:43:15+00:00 optional_format_in_resource_routes2.patch 64878 12850 37059 http://rails.lighthouseapp.com/attachments/64878/optional_format_in_resource_routes2.patch