Skip to content

Fix bytes.decode() and str(b, encoding) looking up the codec for empty input - #1227

Open
jseop-lim wants to merge 1 commit into
oracle:masterfrom
jseop-lim:GH-1223-decode-empty
Open

jseop-lim wants to merge 1 commit into
oracle:masterfrom
jseop-lim:GH-1223-decode-empty

Conversation

@jseop-lim

Copy link
Copy Markdown
Contributor

Description

Fixes #1223

BytesCommonBuiltins.DecodeNode, which backs bytes.decode, bytearray.decode and str(obj, encoding), always looked up the codec, so an empty input with an unknown encoding raised LookupError. CPython's bytes_decode and bytearray_decode go through PyUnicode_FromEncodedObject, which returns '' for empty input without a codec lookup outside dev mode (unicodeobject.c#L3311-L3317). GraalPy's own PyUnicodeFromEncodedObject already has the same shortcut (PyUnicodeFromEncodedObject.java#L85-L87), so C extensions and Python code disagreed.

AS-IS

>>> b''.decode('bogus')
LookupError: unknown encoding bogus
>>> str(memoryview(b''), 'bogus')
LookupError: unknown encoding bogus

TO-BE

>>> b''.decode('bogus')
''
>>> str(memoryview(b''), 'bogus')
''

(matching CPython)

Changes

  • Return T_EMPTY_STRING from DecodeNode.decode when the bytes storage is empty, behind an InlinedConditionProfile, as PyUnicodeFromEncodedObject.doBytes does.
  • Add test_decode_empty_skips_codec_lookup to tests/test_bytes.py.

Testing

mx python-jvm on linux-aarch64, then mx graalpytest test_bytes.py test_codecs.py test_string.py (237 passed). test.test_codecs, test.test_str and test.test_bytes from lib-python give the same per-test results as on master (on the jvm-ce builds, where BytesAsStringTest.test_adaptive_find alone takes about 40 minutes on both). The new test fails on the unpatched build. Non-empty input still raises LookupError for an unknown encoding, and a non-buffer still raises TypeError.

For performance, I built the jvm-ce standalone (with the compiler) from master and from this branch and timed 1M-call loops after warmup, alternating the two builds over five runs. On a 52-byte input, bytes.decode('utf-8'), bytes.decode('latin-1'), str(b, 'utf-8') and bytearray.decode('utf-8') stay within 3 ns of master at 30 to 45 ns per call, which is inside the run-to-run spread. b''.decode('utf-8') drops from 31 ns to effectively zero.

BytesCommonBuiltins.DecodeNode, which backs bytes.decode,
bytearray.decode and str(obj, encoding), always went through the
codec, so b''.decode('bogus') and str(b'', 'bogus') raised
LookupError.

CPython's bytes and bytearray decode call PyUnicode_FromEncodedObject,
which returns '' for empty input without looking up the codec outside
dev mode. GraalPy's own PyUnicodeFromEncodedObject already does this
for the C API; DecodeNode now has the same shortcut.

Signed-off-by: Jeongseop Lim <jeongseop_lim@korea.ac.kr>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OCA Verified All contributors have signed the Oracle Contributor Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: str(b'', encoding) raises LookupError for an unknown encoding instead of returning ''

1 participant