sprintf.js icon indicating copy to clipboard operation
sprintf.js copied to clipboard

Prevent arguments leak

Open Landmaster opened this issue 7 years ago • 2 comments

In the original code, the arguments object is leaked, which disables optimization in the V8 engine. This pull request rectifies this by manually copying the passed-in arguments to a new array, keeping arguments intact.

Landmaster avatar Jun 27 '17 03:06 Landmaster

I don't think extra code really worth it. What kind of performance hit are we even talking about? Can you measure when does it make a difference?

Also if we drop support for even more old browsers, we can just write something like this:

sprintf(key, ...argv) {

nazar-pc avatar Aug 06 '17 02:08 nazar-pc

Well, depends a lot in the amount of consecutive calls to sprintf that you do. I'm maintaining a program that handles tens of thousands of inputs and this is a neat performance hit.

I think that avoiding V8 deoptimizations is a good practice. Actually preventing V8 from optimizing functions is a really bad practice, as you don't actually know what V8 do in the back-end and if in the future they are going to add new ways of optimizations that this code will prevent.

Llorx avatar Dec 03 '18 12:12 Llorx