19 Reasons Why It Is Not "Fast"
Closed this issue · 1 comments
anonyco commented
The concept of performance and npm package is a nice sentiment, but there are so many different issues with this library. Let me list a few.
Function.prototype.apply
breaks when passed an array that is too big. In the console of Chrome 74, this limit appears to be 125833:[].push.apply([], (new Array(125833)).fill(0))
Array.prototype.push
+String.fromCharCode
= performance bottleneck. Use binary string concatenation if you must accumulate a string.- Stop reassigning the typed array
target
in theFastTextEncoder.prototype.encode
function. Even if this does not happen often, the mere presence of reassignment forces the JIST compiler to insert extra code in case of recompilation due to potential constructor changes oftarget
. More code means that aligning to the page line is less likely. See this SO post.. - In
FastTextDecoder.prototype.decode
, the assertion thatif (byte1 === 0) break;
is completely wrong because you should use a null byte ("\x00"
) instead. - In
FastTextDecoder.prototype.decode
, you should try to reuse the underlying buffer instead of copying everything into a new duplicate array in case if a typed array is passed instead of an ArrayBuffer. - You should support NodeJS<3.0's native Buffer and use it as an alternative to typed arrays.
- Stop using rest parameters. Minifiers are horrible at bloating the code size on account of them. Use the
||
operator instead to ensure greater standards compliance and a smaller file size at the minimal overhead of performance. - In the browser, you should use
Array
as a shim-like fallback for TypedArrays to provide limited backwards support into IE5.5-IE9. - Bring in the new and the shiny
"use strict"
, which your minified file (the only file that actually counts) does not do. - Test
TextEncoder
andTextDecoder
separately. There is simply a right way to do polyfilling and there is simply a wrong way to do polyfilling. Except with vendor-specific features, you should never assume that just because one item does not exists, the other one does not exist too. - Apply SMI integer optimizations to lines 55, 59 x2, 63 x2, 67, 69, 70, 79, 80, 81 x4, 90, 93, 95, 96, 98-100, 106, 144, 148 x2, 156, 159, 160, and 163-165 to maximize performance.
- Use
!==
instead of!=
when comparingtypeof
. The browser is able to assume that it will be a string-only comparison, thus the extra byte used is a waist of space. - If you must use
Object.defineProperty
, then use try/catch aroundObject.defineProperty
with alternative code for IE5.5-IE8. The try/catch is becauseObject.defineProperty
exists only for DOM in IE8. - Stop using
new
in front ofError
/RangeError
. It is not needed and it waists precious space for something that is not a performance priority in the slightest. - Support AMD.
- Add in support for Service Worker usage. Neither
window
,global
, northis
(in strict mode) are available in Service Workers. Useself
instead ofwindow
to support the Service Worker environment instead of relying on your minifer to strip the"use strict"
so thatthis
can be used. - Write to
module.exports
if available for drop-in use in NodeJS. - Use a different minifier. Your current minified code reassigns argument variables corresponding to what was originally rest parameters, thus greatly diminishing the stack overhead optimizations that the V8 JIST compiler is able to apply to the function and delaying entry into the function.
- Replace
"undefined"
with""+void 0
in the minified code to save 2 bytes of space with each occurrence without performance penalty.
I came up with my own original solution that fixed many problems, greatly increased performance, and reduced file size.
FastestSmallestTextEncoderDecoder
samthor commented
Thanks for the feedback. At least one of your points will make it into a release.