Template vs Context API

I really love the current Tilt api because it quite simple, flexible,
and has a clear caching story. Tilt emphasizes templates as the main
model. So its very easy to grasp.

template = Tilt::ERBTemplate.new("foo.erb")
context = Module.new
template.render(context, :foo => "bar")

*Templates are rendered inside contexts*

A context has no requirements. It could be a controller, a template
instance, or even a model record. Super flexible, no contract. Well,
it just depends on "instance_eval" which all Objects support.

Template instances are responsible for their own cache. If a template
is stale, needs to be reloaded or whatever, just drop the reference.
'cache["foo"] ||= Tilt::ERBTemplate.new("foo.erb")' is a viable
caching strategy.

Flip-Flopped Context Focused API
--------------------------------

This is how we do template compiling caching in Rails. I loathe it.
However, its quite fast ;)

We could break alway from generic contexts (just any object) and
require them to implement some behavior. Contexts become the main
model.

*Contexts render templates*

class Context
include Tilt::CompiledTemplates
end

template = Tilt::ERBTemplate.new("foo.erb")
context = Context.new
context.render(template, :foo => "bar") # internally compiles (if
necessary) and calls cached __render_foo_erb

* Contexts have a stricter definition and can't be any object. So you
need to be sure you are rendering against some "complaint" context.
Tilt needs to provide "Tilt::Template" and "Tilt::Context". (twice as
many as one simple class)

* Templates along with their locals need to be hashed into some sort
of unique key to be used as a method. Context method pollution is
another minor concern.

* Contexts hold a set of cached templates. If difficult and not
completely clear how to expire single templates. This usually leads to
memory leaks if you are using one off templates and throwing them
away. Expiring template requires some sort of Tilt.expire(context,
template) api, you can't just forget about it. Template lifespan is
now a prominent concern people need to be more aware of.

----

I have issues with the design philosophy of the context focused
approached. I mainly complaining about using "CompiledTemplates" as a
public API. It doesn't really matter to me how Tilt templates work at
the end of the day. If we could somehow implement the module caching
approach internally and keep the same simple Tilt API, I'm all for
that.

Nice write up. I couldn't have put it better myself.

I think reorganizing Tilt to be context focused would be a mistake --
better to just create another project that went at it with that
approach.

I remember having a conversation with Yehuda about this at RubyConf. I
didn't fully understand what he was talking about at the time, but now
that you've laid this out it makes a lot more sense. He suggested
making the compiled templates module an option. There would be some
way a module object could be passed to Template::new (or configured
globally) to hold the pre-compiled template methods. Then, in
Template#render, the provided context object would be extended with
the configured CompiledTemplates module and the pre-compiled template
method invoked.

Some more or less random thoughts on this:

1.) If you want the performance benefits of pre-compiling, you'd need
to do a little extra work in providing the CompiledTemplates module.
That shouldn't be an issue for frameworks like Rails or Sinatra where
pretty much all template managing code is in once place.

2.) By providing a CompiledTemplates module explicitly, you're
acknowledging that your context objects will be extended. I did not
like the idea of extending context objects all willy nilly as the
default behavior but I feel better about it in this scenario where you
have to opt in.

3.) Extending the context object with the CompiledTemplates module on
each call to render may be rather expensive. Maybe we check if the
context object respond_to? the compiled template method, and if so,
assume that the module was already included. This way, Rails can
include its CompiledTemplates module in the base View class; Sinatra
would include the CompiledTemplates module in Sinatra::Base.

4.) The problem of cleaning up / expiring old template methods is
punted to whoever provides the CompiledTemplates module. A simple
strategy may be to simply recreate the module every N requests, or
once the number of methods exceeds some threshold.

With this approach, it seems like we could keep the template focused
interface, but still get the performance benefits of precompiling,
although maybe not as seemlessly as with a context focused approach.

Thoughts?

Thanks,
Ryan

Ideally, you don't have to think about this.

Cleanup is the dirtiest part. Memory leaks are a serious concern.

This discussion could easily go out of control when more people chime
in. Lets make sure we are focusing on real implementations.

http://gist.github.com/320391

Option #2 is how you described.

Option #1 is interesting. Tilt could (even optionally) include
CompiledTemplates into Object making any object a valid context. The
cons are "method pollution" on every object. It seems like a valid
concern, but I'm more interested if there are any performance hits for
shoving this into every object. Finalizers should automatically deal
with unused complied methods. The real win is keeping the Tilt api the
same. Maybe one day, Ruby will be fast enough not to need this
compiled method hack and we can revert to instance eval or whatever.

--
Joshua Peek

I don't disagree. But if the code to perform cleanup can be simple
outside of tilt while it must be complex inside of tilt, I'd prefer to
punt it to the outside and let the calling code handle it. I'm
assuming cleanup would require a lot of logic inside of tilt, though.
There may be a clean and simple way to accomplish it.

I was thinking of something slightly different. I'll get a branch
together tonight or tomorrow.

Having to include these pre-compiled template methods in Object would
make me cry. You're trying to send me back to Python, I think ;)
There's got to be a better way. Even if there's no performance issues,
I'd still hate to do that if only because it would be a mess listing
methods on objects in IRB and ruby-debug.

Here's a slight modification to option #1 that extends the context
object if the precompiled method doesn't exist:

http://gist.github.com/320420

Yehuda tells me extending objects breaks the method cache so this
could be more trouble than it's worth.

I still like the idea of making the calling code own the module. That
way it can be responsible for including it in context objects.

Just so I'm clear, the idea with using finalizers would be to
automatically remove the method from the module when the Template
instance falls out of reference? That seems like a good idea
regardless of who owns the module.

That reminds me. I'm not entirely sure using a proc +
instance_eval/instance_exec won't work here. Using a method is
supposed to be a little faster but I don't think it's a huge
difference. The reason that approach was reverted was because it broke
under MRI 1.9.2 due to the arity changes with lambda or something. I'm
not convinced that approach is fundamentally flawed. Before we get
ahead of ourselves going the module/method route, we should make sure
there's not a cleaner way.

I'm going to create a few branches with different approaches and maybe
run some benchmarks.

Thanks,
Ryan

My bad. It was yield from within a template that broke for some reason
under 1.9:

1) Error:
test_passing_a_block_for_yield(ERBTemplateTest):
LocalJumpError: no block given (yield)
(__TEMPLATE__):1:in `block in template_proc'
/Users/rtomayko/git/tilt/lib/tilt.rb:235:in `instance_eval'
/Users/rtomayko/git/tilt/lib/tilt.rb:235:in `evaluate'
/Users/rtomayko/git/tilt/lib/tilt.rb:112:in `render'
/Users/rtomayko/git/tilt/test/tilt_erbtemplate_test.rb:34:in
`block in <class:ERBTemplateTest>'

It breaks under MRI >= 1.9.1. I'm pretty sure we can figure out a fix.

Thanks,
Ryan

I did a little benchmark of this file: http://github.com/judofyr/parkaby/blob/master/bench/simple.erb,
and the method approach turned out to be 8 times faster than
instance_eval with a string.

I'm looking at this in a more practical way: Imagine you're writing
the most awesome web framework, and you're writing most of your
templates with ERB. Would you choose Tilt which supports tons of
template engines or would you just implement ERB support yourself and
make template rendering 8 times faster? Unfortunately, I believe many
would take the "performance-route" just to have some shiny numbers in
their announcement.

And the "Custom template evaluation scopes" part of Tilt is nice, but
in reality the context will almost always be some instance of a
Controllers::Base-class and we therefore have a very sensible place to
define the methods (not Object, please).

That said, it's probably possible to keep support for both approaches
without too much work.

As far as I know, proc + instance_eval/instance_exec won't work
because if we're caching it we can't capture any yields within it, and
currently you can't invoke instance_eval/instance_exec with a block.
I'd love to be proven wrong though :-)

On Mar 3, 9:14 am, Ryan Tomayko <rtoma...@gmail.com> wrote:
> On Tue, Mar 2, 2010 at 11:11 PM, Joshua Peek <j...@37signals.com> wrote:

Yeah. There's a pretty massive perf gain to be had not needing to
parse the source into its nodes each time. Lots of needless object
creation is avoided too. I think the question now is how much better
methods perform vs. procs.

Well, Tilt is just an extraction from Sinatra. We were getting tons of
requests to add support for different template languages and didn't
want to take that on as a core piece of Sinatra so we split it out.
The other big motivator for me was that it's always a pain in the ass
trying to get backtraces w/ correct line numbers and sane scope rules
with every template engine I've dealt with. I wanted to know that as
long as I conform to an interface, the engine will do basically what I
want.

Also, great performance is going to be one more reason to choose tilt.
It's not there yet but I'm confident we'll get it nailed down and then
that's just one more benefit you get simply by following a standard
interface.

Another place I use tilt and plan to use it a lot more is generating
static files. For instance, most projects I work on usually end up
with a bunch of markdown files + an ERB template under a `doc/`
directory. It's trivial to throw a few Rake tasks together that
generate the content. In those cases, the scope is typically a simple
Hash or even just Object. So I'd like tilt to stay useful in non-web
framework cases as well. I definitely don't want to assume the
existance of a common controller/view object.

I do agree that in cases where performance is critical, you're likely
going to have a common base class for context objects and that we
shouldn't ignore this if we have to go the method route.

I'd love for that to be proven wrong too :) I'm playing with it now
and it looks like it might be impossible under 1.9. More soon.

Thanks,
Ryan

"Object.include Tilt::CompiledTemplate" was just a crazy idea that
could work. 'AnyObject.methods.grep(/tilt/) #=> lots' is nasty

Right, extend on the fly is no good.

I put together a #3 sample that combines my first two.

http://gist.github.com/320391

We keep the instance_eval path around so we can eval on any object. If
you know ahead of time some class is going be reused over and over as
a context object, you can opt-in for more speed by including
CompiledTemplates. The caching contract stays the same. The template
is responsible for cleaning up its compiled method cache when its
GC'd.

The cleanup and finalize approach is very cool (if i do say so
myself). I hope there is no technically implementation issues with
doing that.

I have branches up for both the proc and method approaches:

procs: http://github.com/rtomayko/tilt/compare/precomp-procs
methods: http://github.com/rtomayko/tilt/compare/precomp-methods

The procs approach falls down under 1.9 due to the yield from proc
problem. I wasn't able to figure out a way around it. Otherwise, I
love it and it would make life a whole lot easier. It works great on
1.8.7.

I plan on merging the precomp-methods branch after adding finalizers.
It looks a lot like #3, allowing the compile site to be configured on
a template-by-template basis but also has a one-line hack to enable
template compilation on Object:

Tilt.enable_global_compile_site!

The way I plan on using it from Sinatra would be something like:

module Sinatra
module CompiledTemplates
end

class Base
include CompiledTemplates

def render(type, name, options, locals, &block)
Tilt.new(type, options, CompiledTemplates) { # load template data }
end
end
end

I assume Rails would be very similar, assuming this meets all your needs.

I'm pretty happy with how this turned out, to be honest. It's a lot
more complex than procs but it seems to be the best we can do and
forced me to clean up some other crap that's been laying around
forever.

Review and feedback appreciated. Huge thanks to everyone who chipped in on this.

Ryan

You know, I do like how the #3 example just uses the mixin to
determine if the scope supports compilation. That'll make things a ton
cleaner.

Thanks,
Ryan

Which branch is that off of? The precomp-methods branch handles it
fine. The precomp-procs branch may use the first block passed forever.
Actually, yeah, I'm sure that's it.

Also, I've updated the precomp-methods branch to look a bit more like
josh's example #3. There's a simple Tilt::CompileSite mixin. Anything
that includes it gets compiled methods when used as a scope.

Thanks,
Ryan

I don't think we need multiple compile sites. One "Tilt::CompileSite"
should work fine. It shouldn't be harmful if some of Rails' compiled
templates get in Sinatra's module. Its no huge deal, just think
compile site should be as internal as possible.

+ "__tilt_#{object_id}_#{locals.keys.hash}"

Probably need a better hashing algorithm for locals.

--
Joshua Peek

We only need uniqueness over key names, not values, so that should be
pretty close. If not, we can crc32 the concatenated keys or something
and that should give us a bit more insurance against collisions.

Somewhat related, I ran into all kinds of weirdness with finalizers
and object_id uniqueness. It appears object_ids can be reused long
before the previous object's finalizers are run, causing all kinds of
strangeness. Or something. I had to add a timestamp to the method name
to avoid collisions.

Ryan

Nice.

Yeah. Good catch. This might be a regression Sinatra 1.0.a now that
you mention it.

I'll see about adding a test that verifies the finalizers aren't
holding onto to anything. I verified they were running with puts
debugging but it'd be good to having some a bit more robust in there
to catch any issues in the future.

Thanks,
Ryan