After completing #2884 and while working on #2878, I realized that I might have somewhat pessimized path evaluation by calling _.isArray more often through _.toPath. Coincidentally, I also noticed that the internal tagTester was needlessly recomputing the [object Type] tag on every invocation. Since the isType functions are used everywhere, I decided to offset the _.toPath pessimization with an optimization across the board.
I then revisited the benchmarks discussed from https://github.com/jashkenas/underscore/pull/2840#discussion_r407696041 onwards, because I suspected the tradeoff for the optimization in _.isEmpty might have changed. Indeed, it had; _.isEmpty({}) still took about three times as much time without the optimization than with it, but in Node.js, _.isEmpty('') took five times as much time with the optimization than without it. I was then able to establish a good compromise between the optimized and the unoptimized performance profiles, by making the optimization itself more efficient. This evaluates obj.length only once instead of twice, which keeps the intended effect of the optimization while also avoiding the performance penalty for strings in Node.js.
This PR is just to leave a trace. The weight does not change significantly and performance is likely to improve for applications that call isType functions inside hot loops. If the CI checks pass, I'll merge this without waiting for a review. Nevertheless, a review by a spontaneous volunteer is still welcome.
Coverage increased (+0.02%) to 95.175% when pulling 3730dfc2f6ff5f83b99edd3ef2e603bfd12b45ba on jgonggrijp:stringtag-perf into 1964cdb872e892f01464797f2df8038f912fd59f on jashkenas:master.
After completing #2884 and while working on #2878, I realized that I might have somewhat pessimized path evaluation by calling
_.isArray
more often through_.toPath
. Coincidentally, I also noticed that the internaltagTester
was needlessly recomputing the[object Type]
tag on every invocation. Since the isType functions are used everywhere, I decided to offset the_.toPath
pessimization with an optimization across the board.I then revisited the benchmarks discussed from https://github.com/jashkenas/underscore/pull/2840#discussion_r407696041 onwards, because I suspected the tradeoff for the optimization in
_.isEmpty
might have changed. Indeed, it had;_.isEmpty({})
still took about three times as much time without the optimization than with it, but in Node.js,_.isEmpty('')
took five times as much time with the optimization than without it. I was then able to establish a good compromise between the optimized and the unoptimized performance profiles, by making the optimization itself more efficient. This evaluatesobj.length
only once instead of twice, which keeps the intended effect of the optimization while also avoiding the performance penalty for strings in Node.js.This PR is just to leave a trace. The weight does not change significantly and performance is likely to improve for applications that call isType functions inside hot loops. If the CI checks pass, I'll merge this without waiting for a review. Nevertheless, a review by a spontaneous volunteer is still welcome.