nodejs / nodejs/node

TextDecoder is wrong and very slow

未关闭
#61,041 14 条评论 11 个 reaction 已指派 0 人 在 GitHub 查看

还没有人认领这个 Issue。

confirmed-bug performance
主要语言
JavaScript
星标
122k
派生
37.4k
平均合并
4 天 3 小时
30 天内合并 PR
272

描述

Correctness

Encodings that return invalid results:

  • Single-byte:
    • ibm866 (fails at even ascii input)
    • koi8-u
    • windows-874
    • windows-1252
    • windows-1253
    • windows-1255
  • Multi-byte (all except gb18030):
    • gbk (should be identical to gb18030 but it is instead broken)
    • big5
    • euc-jp
    • iso-2022-jp
    • shift_jis (fails at even ascii input)
    • euc-kr

Unimplemented encodings that throw:

  • iso-8859-16
  • x-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) TextDecoder is much slower on ascii input than it can and should be
    1.3x on 4096 bytes, ~3x on 1 MiB input
  • The above applies to buffer.toString() too
    It's much slower on ASCII input than a checked js impl (same 1.3x-3x)
  • windows-1252 aka new TextDecoder('ascii') aka new TextDecoder('latin1')
    is ~2x-4x slower than an optimized impl on ascii input
  • windows-1252 aka new TextDecoder('latin1')
    is ~6x-12x slower than an optimized impl on latin1 input
  • windows-1252 is ~7x-12x slower 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-1252 are >=10x slower than the js impl on ascii input
    (windows-1252 is only ~2-4x slower)

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

  1. Add a proper ASCII fast path to buffer.toString()
    https://github.com/nodejs/node/pull/61119
  2. Add a proper ASCII fast path to new TextDecoder().decode(arg)
    https://github.com/nodejs/node/pull/61119
  3. 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
  4. Remove gbk decoder path and make it do the same as gb18030 as the spec says
    https://github.com/nodejs/node/pull/61099
  5. For utf16 decode optimistically using existing fast apis, then check the string for validity
    https://github.com/nodejs/node/pull/61559
  6. 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
  7. 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

贡献指南

打开贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

调研方向

从 issue 中描述的 TextDecoder 和 buffer.toString() 入口点开始,然后运行所引用的字节编码测试,以重现一个具体的正确性故障,或对一个具体的性能案例进行基准测试。将工作范围限定为一种编码或一个 fast path,并将完成定义为相关正确性测试通过,或证明达到目标改进,同时不改变无关的 decoder 路径。

由索引模型根据 Issue 内容生成。

评估

技术栈
javascript, nodejs
领域
backend, performance
Issue 类型
缺陷
难度
5/5
预计耗时
一周以上
活跃度
冷清
描述清晰度
需要澄清
新手友好度
25/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。