Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Visitor holding Value #53

Open
Keats opened this issue Jun 4, 2021 · 13 comments
Open

Visitor holding Value #53

Keats opened this issue Jun 4, 2021 · 13 comments

Comments

@Keats
Copy link

Keats commented Jun 4, 2021

As requested on the discord, posting a detailed issue about trying valuable for Tera.

Background

I am trying to see if valuable could replace serde_json::Value for Tera.
The goal in a template engine is to find the value the user wants to use in a given context.
Right now, the context in Tera is just serde_json::Value, which means we serialise everything even though we don’t really need it. See https://docs.rs/tera/1.10.0/tera/struct.Context.html for examples of Context.

In Tera, you need to evaluate expressions based on the context. For example:

  • {{ get_metadata(path=page.path ~ “image.jpg”) }}: this concatenates into a single string which is then passed as an argument to a function
  • {% if user.is_admin or data.public %}: this evaluates the fields truthiness
  • {{ item.price * current_rate | format_currency }}: this multiplies the value found in the context and then applies a filter on the output
  • {% for result in data.results[user.birth_year] %}

There are more examples on the Tera documentation: https://tera.netlify.app/docs/
If you are familiar with Django/Jinja2/Twig/Liquid this is essentially the same thing.

The way it currently work is the following:

  1. Convert the whole context received into serde_json
  2. Evaluates values in brackets, like user.birth_year in the example. If the value is an integer, we assume it’s the index of the array so we don’t change anything
  3. Replace the values in brackets by their actual value and create a JSON pointer based on it
  4. Use https://docs.serde.rs/serde_json/enum.Value.html#method.pointer on the context to retrieve the value (or not if it wasn’t found)
  5. Do pattern matching on the Value kind: if we’re doing a math operation like *, we need to make sure both operands resolve to a Value::Number for example, string concatenation with ~ only work on Value::String and so on

Where valuable comes in

As mentioned, it currently serialises the whole context which can be very costly. For example, rendering a section in a Zola site will clone the content of all the sub-pages if pagination isn’t set up, which can quickly become slow.
The whole Tera cloning the context is actually one of the biggest current perf bottleneck in Zola as you need to clone the content every time you render the page, which can be pretty big considering it’s potentially thousands of HTML pages.

Examples

What I’d like to accomplish is be able to visit the context and extract the Value for each identifier. Let’s use https://github.com/tokio-rs/valuable/blob/master/valuable/examples/print.rs as an example data and say that the person variable is our context.

{{ name }}

The path is [“name”] so we only need to visit Structable and Mappable and look for that key

{{ addresses.1.city }}

The path is [“addresses”, 1, “city”]. We visit Structable/Mappable because it begins with a string and visit the field where it is equal to addresses. This is an array so we visit Listable. We know we want to only care about the element at index 1 so we could only visit that one ideally (see #52). Once there, there is only city left to match so we visit in Structable/Mappable again.

{% for address in addresses %}

Here we want to iterate on the Vec<Address> so I think we just need to visit the addresses and get a Vec<Structable> which we visit in its entirety one by one (since you can break from the loop).

The goal

In all cases above we want to get the Value corresponding to the path out of the visit and pattern match later. Since visit doesn’t return anything, I need to store it on the visitor struct. The Value found could be a primitive, a Mappable/Structable (for loops on key values) or not found at all.

However I get a lifetime error if I try to assign a Value in a small toy example: https://gist.github.com/Keats/150acda601910c5f8a6bd9aceba2ddfd so I’m not sure how I would accomplish that. Is there an example of a visitor holding a Value somewhere?

The article at https://tokio.rs/blog/2021-05-valuable specifically mention template engine so I’m wondering if the idea was limited to writing the value to a string buffer or whether it is too limited for Tera.

@carllerche
Copy link
Member

{% for address in addresses %}

For most of this, it seems like we need a "pointer" concept similar to a JSON pointer, which should be totally doable as an additional trait fn.

Here we want to iterate on the Vec<Address> so I think we just need to visit the addresses and get a Vec<Structable> which we visit in its entirety one by one (since you can break from the loop).

I'm not following this, why does {% for address in addresses %} require collecting all the addresses into a Vec instead of rendering each partial in the visitor.

@Keats
Copy link
Author

Keats commented Jun 11, 2021

I'm not following this, why does {% for address in addresses %} require collecting all the addresses into a Vec instead of rendering each partial in the visitor.

You have a few specific variables available in a loop that require knowing the length and the current index of the item and (not yet implemented in Tera) a variable holding the previous and next item: https://jinja.palletsprojects.com/en/3.0.x/templates/#for

@carllerche
Copy link
Member

Hmm, Ok. It may be worth considering switching Listable to be fixed length.

Is loop.changed(*val) relevant here?

@Keats
Copy link
Author

Keats commented Jun 11, 2021

Is loop.changed(*val) relevant here?

Not for Tera at least

@carllerche
Copy link
Member

@hawkw @taiki-e thoughts on if Listable should be fixed-sized?

@taiki-e
Copy link
Member

taiki-e commented Jun 16, 2021

thoughts on if Listable should be fixed-sized?

What does fixed-sized list in the context of valuable mean? How does it differ from matches!(size_hint(), (x, Some(y)) if x == y) listables?

@carllerche
Copy link
Member

@taiki-e the main difference is that it would not be permitted to have an unknown length. i.e. instead of targeting any Iterator we would only permit ExactSizeIterator.

@sunng87
Copy link
Contributor

sunng87 commented Jul 11, 2021

Actually in template/expression engine we might not use the visitor pattern for all cases. I prefer as_value and Value APIs for path navigation. Does it make sense to add methods like get_by_key and get_by_index for Mappable and Listable?

@Keats
Copy link
Author

Keats commented Jul 11, 2021

With #59 working with strings, it should work well.

@sunng87
Copy link
Contributor

sunng87 commented Jul 11, 2021

As long as Mappable's visit api is implemented like this, it always take O(n) to access a field using visitor API.

@carllerche
Copy link
Member

@sunng87 for fast lookups, using Structable is better than Mappable. Note how Structable supports dynamic fields. The idea is, if you know fields you want to lookup "fast", define them as a structable, then have the rest of the fields as dynamic fields.

@carllerche
Copy link
Member

get_by_key is not really possible to implement for Mappable w/ generic key types for HashMap (as far as I could tell).

@sunng87
Copy link
Contributor

sunng87 commented Jul 13, 2021

get_by_key is not really possible to implement for Mappable w/ generic key types for HashMap (as far as I could tell).

I see. Thank you for clarification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants