samthor/fast-text-encoding

19 Reasons Why It Is Not "Fast"

Closed this issue · 1 comments

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.

  1. 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))
  2. Array.prototype.push + String.fromCharCode = performance bottleneck. Use binary string concatenation if you must accumulate a string.
  3. Stop reassigning the typed array target in the FastTextEncoder.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 of target. More code means that aligning to the page line is less likely. See this SO post..
  4. In FastTextDecoder.prototype.decode, the assertion that if (byte1 === 0) break; is completely wrong because you should use a null byte ("\x00") instead.
  5. 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.
  6. You should support NodeJS<3.0's native Buffer and use it as an alternative to typed arrays.
  7. 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.
  8. In the browser, you should use Array as a shim-like fallback for TypedArrays to provide limited backwards support into IE5.5-IE9.
  9. Bring in the new and the shiny "use strict", which your minified file (the only file that actually counts) does not do.
  10. Test TextEncoder and TextDecoder 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.
  11. 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.
  12. Use !== instead of != when comparing typeof. The browser is able to assume that it will be a string-only comparison, thus the extra byte used is a waist of space.
  13. If you must use Object.defineProperty, then use try/catch around Object.defineProperty with alternative code for IE5.5-IE8. The try/catch is because Object.defineProperty exists only for DOM in IE8.
  14. Stop using new in front of Error/RangeError. It is not needed and it waists precious space for something that is not a performance priority in the slightest.
  15. Support AMD.
  16. Add in support for Service Worker usage. Neither window, global, nor this (in strict mode) are available in Service Workers. Use self instead of window to support the Service Worker environment instead of relying on your minifer to strip the "use strict" so that this can be used.
  17. Write to module.exports if available for drop-in use in NodeJS.
  18. 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.
  19. 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

Thanks for the feedback. At least one of your points will make it into a release.