search

xslate / p5-Text-Xslate

Scalable template engine for Perl5
https://metacpan.org/release/Text-Xslate
Other
121 stars 47 forks source link

Moo conversion #178

Open yanick opened 7 years ago

yanick commented 7 years ago

Rebase of #140, with a few optimizations that was discussed in its comment threads.

karenetheridge commented 6 years ago

Would it be possible to rebase this down to one or a smaller handful of commits?

yanick commented 6 years ago

Sure. Let me see what I can do.

yanick commented 6 years ago

There we go.

jomo666 commented 6 years ago

Please, don't touch the current Xslate with Moo. Fork some new Mooslate or such. Any (even small) speed drop is unacceptable. Moo--.

karenetheridge commented 6 years ago

@yanick I don't think your force push took effect on the server.

I share the concerns about performance -- some benchmarking is essential to be sure that there isn't a change. I know Moo can be high-performance, but a few things need to be done to ensure that -- e.g. adding in XS accessors.

karenetheridge commented 6 years ago

Looks like there is some work to do to get appveyor to pass, as well. (missing prereq declarations?)

It would also be nice to see this tested on travis. this is a good place to start as a template - https://github.com/p5sagit/JSON-MaybeXS/blob/master/.travis.yml

yanick commented 6 years ago

@karenetheridge

I don't think your force push took effect on the server.

What do you mean? Which server are we talking about?

karenetheridge commented 6 years ago

The github server - it doesn't appear that the squash took effect.

karenetheridge commented 6 years ago

140 contains more discussions re benchmarking. As @shadowcat-mst said, we do not expect any performance degradation with Moo, so if there is any, it is something we can solve with the use of a profiler.

yanick commented 6 years ago

The github server - it doesn't appear that the squash took effect.

Ah, no, it did. I just didn't squashed to a single commit. I kept the part of the history that I thought could be useful, as well as the distinction between my commits and Charlie's.

karenetheridge commented 6 years ago

ok. anyway, it's more important that we get this working with no performance degradation :)

yanick commented 6 years ago

[performance] I'm conferencing as we speak, but as soon as I have some time I'll rerun the benchmarks and see what they reveal.

yanick commented 6 years ago

Appveyor is passing, yay! \o/

yanick commented 5 years ago

Performance: so I looked at the benchmark folder, saw there is no direct way to compare different versions of the code, so I got lost in the yak forest and wrote this http://techblog.babyl.ca/entry/benchpress

This being said, I ran benchmarks for interpolate and demo_tt. Some of the results are at http://techblog.babyl.ca/entry/benchpress/files/stats.html, and so far it seems that the interpolate results are within 7% of the performance of the previous version. The results for demo_tt show roughly a penalty of 10% compared to the previous version.

jonassmedegaard commented 3 years ago

more than two years later - what is the reason this hasn't been merged?

yanick commented 3 years ago

Probably slipped between the cracks, or the 10% speed penalty is too high a price to pay. Might be interesting to replay the benchmarks now, as time and versions passed.