Replacing array push/join with string concatenation

[bev-on]

First of all, thanks folks for amazing work, EJS is great!

I have this idea which I think has potential to increase EJS performance even more.

I never looked at EJS code but I was testing ejs.compile with client:true option and noticed that generated function is using array push and join instead of string concatenation. AFAIK, string concatenation is much faster in all modern browsers (even in Internet Explorer after IE7).

I made small test on jsPerf https://jsperf.com/ejs-array-join-vs-string-concatenation/ which shows huge performance gain using string concatenation.

MS IE 11 & Edge (~3x faster) Opera 50 (~700x faster) Safari 10 (~1000x faster) Chrome 64 (~700x faster) Firefox 58 (~3000x faster)

Most of mobile browsers I tested are performing almost same as their Desktop versions. Also since Node.js is using same V8 engine as Chrome, server side performance gain will be similar.

I don't had time to write more advanced tests but I guess results will be similar.

[bev-on]

I added quick fix which will replace array push/join with string concatenation if you will consider to implement my idea https://github.com/mde/ejs/commit/ad79b9c150ff229d2b08cf7956c54a86d39aaa56

Please let me know if I'm doing something wrong, or maybe there is any benefits of using push/join I'm not aware of.

[mde]

We actually started with string concat, and changed to array push because of performance reasons. :)

As the optimization techniques in JS runtimes have evolved over time, best practices have also changed. What might be optimal in one runtime at a particular time, might be sub-optimal in another runtime, or even in the same runtime a few years later.

How confident are you of the delta in performance, even at very large template sizes? Seems worth making the change if there are big perf wins.

[bev-on]

I will write extensive test for Node.js in near future and share code and results here. I think proper way to test performance in Node.js will be 2-3MB template and at least one million requests with injected random values as properties. I prefer testing in Node.js because I think server side performance is more important (users won't notice 5ms delay in their browsers but this kind of difference can significantly reduce/increase server load on servers with extremely high loads).

[mde]

We already have a pretty extensive set of benchmarking tests, which I have finally updated to work with ESLint:

https://github.com/mde/ejs/blob/master/benchmark/bench-ejs.js

They can be run with node benchmark/bench-ejs.js. Let's start with those. What does the string-concat approach look like under those tests?

Would be great to get more extensive perf testing, but let's use the existing ones as a starting point.