Reuse `RenderContext` after executing partial

[gemhtcr]

For now, because of the following line of code, handlebars doens't share RenderContext when rendering partial.

https://github.com/sunng87/handlebars-rust/blob/9d7d5556287e31e4148841a56e0981b64a679fb6/src/partial.rs#L61C9-L61C39

  // Rust
 let partial = find_partial(rc, r, d, tname)?; 
  
 if let Some(t) = partial { 
     // clone to avoid lifetime issue 
     // FIXME refactor this to avoid 
     let mut local_rc = rc.clone();    // <---  This line can't reuse `RenderContext` after partial
  
     // if tname == PARTIAL_BLOCK 
     let is_partial_block = tname == PARTIAL_BLOCK; 
  
     // add partial block depth there are consecutive partial 

Is it possible to fix it and have a universal RenderContext when rendering ?

[sunng87]

Yes because partial has its own context data. I'm curious about your use case for reusing the RenderContext.

[gemhtcr]

Hi @sunng87 , thanks for your update

My case is that I have an assign helper to assign things to some variable. See below

// _partial.hbs
...
{{#assign "foo"}}
bar
{{/assign}}
...

// parents.hbs 
...
{{> _partail}}
...
{{foo}}       <---- expected to show "bar"

After spending time tracing source, it seems to me the closest solution is to add it into RenderContext so that it can be seen by parent context I'm happy to learn if there is other way to achieve this

[sunng87]

I think it's not possible also it's not recommended to have data modification in templates, which has poor debug experience. I will suggest you to prepare the data ahead of rendering and feed to the template directly.

[cmrschwarz]

Regardless of the original usecase given in this issue, that clone is a major performance issue.

Unfortunately the relevant lifetimes are very deeply engrained into the system, so changing this would cause quite a few breaking changes.

I threw together a proof of concept refactor over here: https://github.com/cmrschwarz/handlebars-rust/tree/avoid_context_clone. This basically eliminates the 'rc and 'reg lifetimes entirely. All the tests pass, but the code would need a bit more love if we actually want to pursue this.

For one my personal usecases, where I have to emit quite a large amount of HTML containing quite a few nested partials, this is a 10x (!) performance increase. (From 42 seconds down to 4.1 seconds for emitting around 90 MB of HTML, I can generate some flamegraphs if anybody cares).

As all the indenting changes will probably require a major version bump anyways, I think this would be at least be worth considering. What's your thoughts on this @sunng87 ?

[sunng87]

This sounds promising to me. Have you tried benchmark with you new implementation?

The current partial has a step of context clone, which I believe is the root cause of slowness.

But although I'm OK with a breaking change and major release, I wonder if it's possible to keep 'reg and 'rc lifetimes while refactoring the partial implementation. This can make upgrade more smooth.