[PATCH] [Optimization] Rack::URLMap clones environment hash
Reported by Chris Heald | March 22nd, 2009 @ 05:44 AM
Very simple little change. Rack currently clones the passed-in environment hash via env.merge, and then passes that to @app.call. #merge! is perfectly applicable here, and prevents an extra copy of the environment hash from hanging around in memory.
Comments and changes to this ticket
-
manveru March 23rd, 2009 @ 03:04 AM
- Tag set to “env, lookup, performance, review, urlmap”
-1
Hash#merge! is no good idea, I would like to defend the status quo. Doing a destructive update of the original env would lead to wrong routing if you are using more than one urlmap in a cascade and the app routed to returns a 404.
So you would go from
/app1/foo, given that there exists anapp1, this part would be cut down to/foo, but if app1 returns 404 and the next urlmap kicks in it won't get the original anymore and repair is very hard.The current Hash#merge comes from one of my patches: http://github.com/rack/rack/comm...
commit df37a917200a6d954292fe00a1b1fe1caa4afcfa Author: Michael Fellinger <m.fellinger@gmail.com> Date: Thu Jan 29 22:34:43 2009 +0900 Non-destructive URLMap -
Chris Heald March 23rd, 2009 @ 05:02 AM
Mmm, makes sense. Chalk this one up to a lack of understanding as to how URLMap could be chained then.
For what it's worth, I discovered this when trying to track down why environment hashes weren't becoming eligible for collection in a Rails 2.3.2 application. I haven't answered that question yet, but I suspect that answer will be far more interesting.
-
manveru March 24th, 2009 @ 03:15 PM
- State changed from “new” to “resolved”
- Assigned user set to “manveru”
OK, I'll close this one for now.
Please Sign in or create a free account to add a new ticket.
With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.
Create new ticket
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป
manveru