Simplify helper wrapper - #2041
Conversation
|
Can you post the setup you are using for measuring the performance improvements? |
|
@jaylinski Posted the setup + the numbers in #1991. For this change, the difference is small, but we drop an extra function call. |
|
I agree that the "wrapHelper" function is a bit redundant. I guess my goal was to also use it in different use-cases. However, I think, "passLookupPropertyOption" (or "wrapWithLookupPropertyOption") is a more descriptive name than just "wrapHelper". So I would keep that name. Another thing is that the runtime.js file is already too large for my taste and I would try to extract code into different files where possible and let the bundler put them back together. In this case, it might be good to extract the whole "addHelpers" or even the four lines where it is called, into a function. Tbh, I never expected function calls to have a lot of impact on JavaScript performance. Since V8 is an optimizing JIT compiler, I always thought it would inline them when necessary. |
b0e52c3 to
668d2fa
Compare
|
@nknapp Good points, updated the PR. Regarding the performance, I agree about the inlining, and I didn't expect to find any difference, but there is something like 5%. Either way, it's hard to tell what exactly the JIT will do so it doesn't hurt to simplify in this case. |
|
Just to get a feeling on how much improvement we have here, I modified you benchmark a little, to only include the "partial" part and to run it multiple times, gathering 10th and 90th percentile of the run time. const Handlebars = require('./dist/handlebars');
const count = 100000;
// Handlebars
Handlebars.registerPartial('partial', Handlebars.compile('1234'));
const template = Handlebars.compile('{{> partial}}');
let times = []
for (let r = 0; r < 100; r++) {
// Warm up
for (let i = 0; i < count; i++) {
template();
}
const start = performance.now()
for (let i = 0; i < count; i++) {
template();
}
const time = performance.now() - start
times.push(time)
}
times.sort((a,b) => a - b)
console.log("10th percentile", times[10])
console.log("90th percentile", times[90])For the 4.x version I got the following numbers: For the current version of this PR: I see that the difference between 10th and 90th percentile can be 4 to 6 percent. There is always this problem with micro-benchmarks, that they have a high variability. This is because they are not deterministic. The computer is doing different things in the background, which might affect the numbers. So my take on this matter is this: I don't think removing function calls does much to increase performance in JavaScript, so I would not remove a function just for performance benefits. But usually, the opposite is true. I sometimes extract functions, even if they are only called from a single location
Sorry for this wall of text. I got pulled into the rabbit hole somehow. |
|
Oh, I didn't see your comment until now. @jaylinski I think this can be merged. But ultimately, it is your choice. |
|
I very much agree with your take, and thanks for verifying the numbers, optimization is definitely a rabbit hole :D |
|
We now have performance benchmarks in |
|
Will re-target to master once #2138 is merged. |
668d2fa to
3898875
Compare
This improves performance slightly and gives the wrapper function a more descriptive name to help with debugging/profiling.
3898875 to
445fb70
Compare
|
Benchmark results: Benchmark Comparison: master vs simplify-helper-wrapperBaseline: /Volumes/MyPassport/Code/handlebars.js/tests/bench/results/bench-2026-04-29T14-14-22-master.md Legend: ! = >2% change, !! = >5% change COMPILATION (Handlebars.compile + first render)
COMPILE OPTIONS COMPARISON
END-TO-END (compile + render)
EXECUTION (template rendering)
PRECOMPILATION (Handlebars.precompile)
|
3 participants
This improves performance slightly and gives the wrapper function a more descriptive name to help with debugging/profiling.