Reduce number of content sequences to speed up compile time #33
Conversation
| #[inline] | ||
| fn render_field_escaped<E: Encoder>( | ||
| &self, | ||
| _hash: u64, | ||
| _name: &str, | ||
| _encoder: &mut E, | ||
| ) -> Result<bool, E::Error> { | ||
| Ok(false) | ||
| ) -> Result<(), E::Error> { |
There was a problem hiding this comment.
Sorry for reviewing so late. I'm using the return values in the #[ramhorns(flatten)] IIRC, so I'm not sure if this breaks things. I'm actually surprised CI passed here.
There was a problem hiding this comment.
As far as I see, only the return values of the Content trait (not ContentSequence trait) are used in the flatten implementation, and those remain unchanged. I can bring ContentSequence return types back if a future use is planned, though.
I just changed the return types because these values were not used anywhere at the moment.
Edit: clarify.
There was a problem hiding this comment.
Ah, that makes more sense, I should have read more carefully!
|
Please, could you submit a new release with this PR merged? |
|
Published now. |
Consider the test example:
In order for it to compile, the recursion limit needs to be raised to
369and then, it takes roughly25sto compile on my machine. That's because the compiler needs to "prepare" for every possible sequence of contents. However, most of them are very unlikely to occur.This PR reduces these by "crawling back" in the content tree when the section is not found in the current content. Effectively, the current content is forgotten and not used in the further content sequences. In other words, the sections in the content sequence always lie on a path in the content tree. All of the previous contents are the ancestors of the current one. This decreases the number of possible content sequences from quartic (the length of the content sequence is 4) to linear, eradicates the need to raise the recursion limit and the compilation is significantly faster: roughly
2.5son my machine.The mustache specification does not mention using sections from the parent contexts at all, so this is kind of an extra feature anyway. All tests are passed and in the majority of cases, the behaviour remains the same. Then, there are some edge differences. Consider the structs from the example.
{{#a}} {{#b}} {{bin1}} + {{ain}} + {{bin2}} {{/b}} {{/a}}now doesn't render
{{ain}}, as it's not found inb, nor in any of the parent contents in the content tree. We can always return to the previous behaviour by using{{#a}} {{#b}} {{bin1}} + {{#a}} {{ain}} {{/a}} + {{bin2}} {{/b}} {{/a}}.This is quite verbose, on the other hand it makes clear in which content the variable is expected.
However, mostly a much simpler solution would suffice:
{{#a}} {{#b}} {{bin1}} {{/b}} + {{ain}} + {{#b}} {{bin2}} {{/b}} {{/a}},or even
{{#b}} {{bin1}} + {{#a}} {{ain}} {{/a}} + {{bin2}} {{/b}}.The only case where a workaround of this sort is not possible is when
aandbare both lists, but my imagination is not sufficient to conceive an actual example of where a construct like that would be desired. I don't think it's required by the specification, as the sections form parent contexts are not mentioned there, and speeding the compilation up, as well as reducing the binary size provides more practical advantages.