Permalink
Browse files

Sort locals_keys before looking up template in @compiled_method

Currently, tilt uses the value of Hash#keys directly as the cache
key.  On ruby 1.8 this results in 2^N possible cache entries for
the template, where N is the number of keys in locals, because
Hash#keys returns the keys in an arbitrary order on 1.8.

Even on 1.9, where Hash#keys returns the keys in entry order,
this can cause similar issues unless user code always enters the
keys in the hash in the same order.

I chose to put this code in evaluate instead of compiled_method
itself, because evaluate creates the array and can use sort!
to sort it.  If done in compiled_method, locals_keys is the
argument, and mutating arguments is generally a bad API practice,
so you'd have to use sort, which would cause an extra array
allocation.
  • Loading branch information...
1 parent a3322c6 commit 4e1b2ead0d4d00591f65de80512e5fd31492fab7 @jeremyevans jeremyevans committed Apr 29, 2015
Showing with 9 additions and 1 deletion.
  1. +9 −1 lib/tilt/template.rb
View
@@ -5,6 +5,8 @@ module Tilt
TOPOBJECT = Object.superclass || Object
# @private
LOCK = Mutex.new
+ # @private
+ SYMBOL_ARRAY_SORTABLE = RUBY_VERSION >= '1.9'
# Base class for template implementations. Subclasses must implement
# the #prepare method and one of the #evaluate or #precompiled_template
@@ -151,7 +153,13 @@ def prepare
# This method is only used by source generating templates. Subclasses that
# override render() may not support all features.
def evaluate(scope, locals, &block)
- method = compiled_method(locals.keys)
+ locals_keys = locals.keys
+ if SYMBOL_ARRAY_SORTABLE
+ locals_keys.sort!
+ else
+ locals_keys.sort!{|x, y| x.to_s <=> y.to_s}
+ end
+ method = compiled_method(locals_keys)
method.bind(scope).call(locals, &block)
end

0 comments on commit 4e1b2ea

Please sign in to comment.