TextDecoder is wrong and very slow
Nadie ha tomado este issue todavía.
- Lenguaje dominante
- JavaScript
- Estrellas
- 122k
- Forks
- 37.3k
- Merge medio
- 4 d 2 h
- PR fusionados (30 d)
- 283
Descripción
Correctness
Encodings that return invalid results:
- Single-byte:
ibm866(fails at even ascii input)koi8-uwindows-874windows-1252windows-1253windows-1255
- Multi-byte (all except
gb18030):gbk(should be identical togb18030but it is instead broken)big5euc-jpiso-2022-jpshift_jis(fails at even ascii input)euc-kr
Unimplemented encodings that throw:
iso-8859-16x-user-defined
If built without icu, utf-16le encoding also returns invalid results:
> new TextDecoder('utf-16le').decode(Uint16Array.of(0xd800))
'�' // correct
'\ud800' // no ICU
Performance
utf-8(aka default)TextDecoderis much slower on ascii input than it can and should be
1.3xon 4096 bytes,~3xon 1 MiB input- The above applies to
buffer.toString()too
It's much slower on ASCII input than a checked js impl (same1.3x-3x) windows-1252akanew TextDecoder('ascii')akanew TextDecoder('latin1')
is ~2x-4xslower than an optimized impl on ascii inputwindows-1252akanew TextDecoder('latin1')
is ~6x-12xslower than an optimized impl on latin1 inputwindows-1252is ~7x-12xslower than an optimized js impl- Other single-byte encodings that are significantly slower than js impl even on non-ascii input:
iso-8859-3,iso-8859-6,iso-8859-7,iso-8859-8,iso-8859-8-i,windows-1253,windows-1255,windows-1257 - None of the single-byte encodings are faster than the js impl even on non-ascii input
- All of the single-byte encodings except
windows-1252are>=10xslower than the js impl on ascii input
(windows-1252is only ~2-4xslower)
References
Nothing of the above requires any changes on the native side, I compared to a somewhat optimized JS implementation
See https://docs.google.com/spreadsheets/d/1pdEefRG6r9fZy61WHGz0TKSt8cO4ISWqlpBN5KntIvQ/edit
See tests in https://github.com/ExodusOSS/bytes/blob/master/tests/encoding/mistakes.test.js (comment out the import and it can be run on Node.js without deps with only that file)
Suggestions
- Add a proper ASCII fast path to
buffer.toString()
https://github.com/nodejs/node/pull/61119 - Add a proper ASCII fast path to
new TextDecoder().decode(arg)
https://github.com/nodejs/node/pull/61119 - Perhaps replace single-byte decoders with a js impl, remove native paths and lib usage. They all are just mappers, the implementation for all of them is identical
Or at least replace the slow, unsupported, or invalid ones.
#61093 - Remove
gbkdecoder path and make it do the same asgb18030as the spec says
https://github.com/nodejs/node/pull/61099 - For utf16 decode optimistically using existing fast apis, then check the string for validity
https://github.com/nodejs/node/pull/61559 - Fix bugs in the non-ICU codepath
https://github.com/nodejs/node/pull/61409
https://github.com/nodejs/node/pull/61549
https://github.com/nodejs/node/pull/61559 - Fix or replace implementations for
big5,euc-jp,iso-2022-jp,shift_jis,euc-kr
To fix legacy multi-byte decoders, attempt to re-use what Chromium has or import js code from@exodus/bytes
Guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Línea de trabajo
Comienza con los puntos de entrada TextDecoder y buffer.toString() descritos en el issue y, después, ejecuta las pruebas de codificación de bytes referenciadas para reproducir un fallo específico de corrección o medir un caso específico de rendimiento. Limita el trabajo a una codificación o fast path, y define que el trabajo está terminado cuando se supere la prueba de corrección relevante o se demuestre la mejora prevista, sin cambiar rutas del decoder no relacionadas.
Escrito por el modelo de indexación a partir del texto del issue.
Evaluación
- Stack tecnológico
- javascript, nodejs
- Área
- backend, performance
- Tipo de issue
- Error
- Dificultad
- 5/5
- Tiempo estimado
- Más de una semana
- Estado de actividad
- Tranquilo
- Claridad
- Necesita aclaración
- Aptitud para principiantes
- 25/100