Speed up filter accesses like {% if a.b.c.d %}.

[rutchkiwi]

Helps a lot with #245, altough I will keep digging further.

I've added an if-bench benchmark, and taken it down from ~145ms to ~30ms.

The main culpit for the slowness was fix-accessor, which was using exception handling
to parse integers. Exceptions are expensive, at least when you use them
a couple of thusand times:

((time (dotimes [_ 100000] (try (Long/valueOf "nan")
                               (catch NumberFormatException _))))
"Elapsed time: 219.507474 msecs"

(time (dotimes [_ 100000] (when (re-matches #"\d+" "nan") (Long/valueOf "nan"))))
"Elapsed time: 11.263704 msecs"

Performance of the if-bench benchmark:

before:

BENCH: Many . acceses in an if clause
Evaluation count : 6 in 6 samples of 1 calls.
             Execution time mean : 145.789759 ms
    Execution time std-deviation : 3.397597 ms
   Execution time lower quantile : 140.769053 ms ( 2.5%)
   Execution time upper quantile : 149.232308 ms (97.5%)
                   Overhead used : 7.470498 ns

after:

BENCH: Many . acceses in an if clause
Evaluation count : 30 in 6 samples of 5 calls.
             Execution time mean : 27.158986 ms
    Execution time std-deviation : 7.895735 ms
   Execution time lower quantile : 21.507726 ms ( 2.5%)
   Execution time upper quantile : 38.751057 ms (97.5%)
                   Overhead used : 7.470498 ns

I suspect that it could be made even faster but I'll need to do more digging, and this is a big improvement in itself.
The other big time spender is add-node it seems, but I haven't looked into it further.

[yogthos]

Ah makes sense, and luckily a straight forward fix for a big performance boost. I can keep the pr open for a bit if you want to dig further, or I can do a release with the fix right away if you need the new artifact on Clojars.

[rutchkiwi]

Thanks! Up to you how you wanna do it - if it's easier for you let's keep this PR open and I will hopefully be able to find more improvements to add to it.

[yogthos]

Sounds like a plan, just ping me when you want to do the release. And thanks for spearheading this! :)

[rutchkiwi]

Sure will do. And no worries, just glad to be able to contribute! :)

…

On 22 Jan 2021, 22:16 +0100, Dmitri Sotnikov ***@***.***>, wrote: Sounds like a plan, just ping me when you want to do the release. And thanks for spearheading this! :) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[rutchkiwi]

Trying this out in my actual project and indeed it takes the rendering time down by 75% or so. But I have two questions that I was hoping you could help me with:

1. Since the change above is about parsing if statement conditions, why does it even do anything when the template cache is on? I thought the template cache stored some kind of AST like structure which had everything parsed and ready? (and thus improvements to parsing shouldn't affect things once it's in the cache). Or does this pre-parsing not go all the way in these cases?

2. What does the add-node fn do, conceptually? I'm feeling a bit lost on the big picture which makes it a bit tricky to figure out whats going on.

I'd really appreciate your help @yogthos :)

[yogthos]

Good to hear. Cache only applies to static elements in the template, but anything with dynamic content has to be recomputed for each render. When the template file is parsed then static content is parsed into strings, and any tags that work with the context map are turned into functions. The resulting template that gets cached is a vector containing a combination of strings and functions here. Then when render is called it runs through the nodes in the vector here to generate the resulting text. The add-node function simply appends a node which is either a string or a function to the vector representing the template.

[rutchkiwi]

Okay thanks. That makes it more clear. I hadn't really figured out how the caching works but now I see that it's done using closures which is cool. I managed to speed things up further but I feel that there is still more to be done so will keep looking.

[yogthos]

Sounds like a plan, looking forward to seeing the changes. :)

[rutchkiwi]

Sorry about the many questions, but is there an easy way to reload changes in the tags NS, like changing the if-any-all-fn function? Right now I have to resort to restarting my repl after every code change. I'm not sure exactly why reloading that file itself doesn't do it.

[yogthos]

Oh yeah, the easiest option is to use tools.namespace to refresh. I just checked in an update on master. If you merge it in you should be able to do (repl/refresh) from the REPL to reload the namespaces.

[rutchkiwi]

Oh wow. That makes it a lot easier. Thanks!

[yogthos]

Thanks for reminding me to add that in. :)

[rutchkiwi]

Okay looks like I've reached the limits of my optimization skills now! Can't really stop any more clear hotspots or easy fixes.

For comparison, the template rendering that powers medino.com's category page used to run in ~850ms, but with these changes its now down to ~150ms. (so about 5x faster).

I'm not sure if that's the limit, 150ms feels pretty long but then again it is a lot of template to render! Perhaps I'll try and revisit at some later point.

Let me know if you spot something funky in the code, if not a release would be much appreciated. :)

[rutchkiwi]

Actually NVM looks like I forgot to check in a test file. (any.html) I'll let you know once it's sorted

[yogthos]

Definitely looks like a solid improvement, and if I get some time I'll take a look to see if some more optimizations are possible. The 5x improvement is definitely nice though. :)

[rutchkiwi]

Okay I've sorted out the missing file and all should work now!

[yogthos]

Fantastic, and just released 1.12.32 with the updates. 🎉

[rutchkiwi]

Thanks a lot! And if you find any more things to speed up, or any ideas that I can look into it'd be much appreciated :)

[yogthos]

Sounds like a plan, and thanks again for doing the PR!