Skip to content

Commit 1623192

Browse files
jbrockmendelclaude
andcommitted
PERF: take the read_csv token boundary from words, not word_starts
Supersedes the _token_len_checked helper from the previous commit, which measured 11.6% slower than main on short-token string columns. GH#66415 computed pass-1 token lengths from word_starts, a second metadata array streamed alongside the words array coliter_next already reads. Taking the boundary from words is identical arithmetic over one array. 123-case csvbench sweep vs main: median 1.0018, 30 cases >=1% faster (top 1.113x), no confirmed regression. Also points the embedded-NUL test at GH#19886, the issue it actually covers, and drops an unsound dtype=object oracle: the C object path interns words by their NUL-terminated prefix, so it is not a valid reference for NUL-bearing data. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 5b0f681 commit 1623192

2 files changed

Lines changed: 34 additions & 40 deletions

File tree

pandas/_libs/parsers.pyx

Lines changed: 19 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2570,9 +2570,9 @@ cdef _string_pyarrow_utf8(parser_t *parser, int64_t col,
25702570
offsets32_ptr[i + 1] = <int32_t>total_bytes
25712571
continue
25722572

2573-
# _token_len_checked, not strlen: an embedded NUL is a data byte
2574-
# here, so strlen alone would truncate the field at it (GH#66277).
2575-
wlen = _token_len_checked(parser, token_idx, word)
2573+
# Not strlen: an embedded NUL is a data byte here, so strlen would
2574+
# truncate the field at it (GH#66277).
2575+
wlen = _token_len_words(parser, token_idx, word)
25762576

25772577
if not large and total_bytes + wlen > <Py_ssize_t>INT32_MAX:
25782578
overflow = True
@@ -3056,40 +3056,26 @@ cdef inline int64_t _token_len(parser_t *parser, int64_t token_idx) noexcept nog
30563056
return <int64_t>parser.stream_len - parser.word_starts[token_idx] - 1
30573057

30583058

3059-
cdef enum:
3060-
# Cap on the _token_len_checked scan: covers typical short string fields
3061-
# while keeping the worst-case scan to half a cache line.
3062-
_TOKEN_SCAN_CAP = 32
3063-
3064-
3065-
cdef inline int64_t _token_len_checked(parser_t *parser, int64_t token_idx,
3066-
const char *word) noexcept nogil:
3067-
# Length for hot loops that already hold the token pointer. Benchmarks
3068-
# (bandwidth-bound parallel string reads) show strlen beating the O(1)
3069-
# boundary arithmetic for short tokens: the scan's byte reads warm
3070-
# exactly the lines the copy pass touches next, and the boundary load's
3071-
# cache miss otherwise lands on the length's dependency chain. For long
3072-
# tokens the scan itself becomes the bottleneck, so those keep the
3073-
# boundary arithmetic (the branch is uniform within a column, hence
3074-
# well-predicted). The next token starts right after this one's NUL
3075-
# terminator, so `end == word + wlen + 1` proves no embedded NUL and the
3076-
# strlen result is exact; on mismatch (embedded NUL, GH#66277) the
3077-
# boundary arithmetic gives the true length.
3078-
cdef:
3079-
int64_t wlen
3080-
const char *end
3059+
cdef inline int64_t _token_len_words(parser_t *parser, int64_t token_idx,
3060+
const char *word) noexcept nogil:
3061+
# Same arithmetic as _token_len, but taking the boundary from `words`
3062+
# rather than `word_starts`. The tokenizer keeps the two in lockstep
3063+
# (words[i] == stream + word_starts[i], rebased whenever the stream
3064+
# reallocs), so the result is identical; what differs is which array the
3065+
# loop touches. coliter_next already loaded words[token_idx] to produce
3066+
# `word`, so the boundary comes off a cache line the loop has in hand
3067+
# instead of streaming a second metadata array alongside the first.
3068+
# `word` must be the unmodified coliter_next result for `token_idx`;
3069+
# an adjusted pointer would silently yield a wrong length.
3070+
# Only the pyarrow string path uses this: rewriting _token_len itself to
3071+
# this form regressed long-token ints ~6% (GH#66277), so the numeric
3072+
# callers keep the word_starts version.
30813073
if token_idx < 0:
30823074
# missing field; word is a static "" outside the stream
30833075
return 0
30843076
if <uint64_t>(token_idx + 1) < parser.words_len:
3085-
end = parser.words[token_idx + 1]
3086-
else:
3087-
end = parser.stream + parser.stream_len
3088-
if <int64_t>(end - word) <= _TOKEN_SCAN_CAP:
3089-
wlen = <int64_t>strlen(word)
3090-
if end == word + wlen + 1:
3091-
return wlen
3092-
return <int64_t>(end - word) - 1
3077+
return <int64_t>(parser.words[token_idx + 1] - word) - 1
3078+
return <int64_t>(parser.stream + parser.stream_len - word) - 1
30933079

30943080

30953081
cdef int _try_uint64_nogil(parser_t *parser, int64_t col,

pandas/tests/io/parser/test_c_parser_only.py

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -856,13 +856,21 @@ def test_block_lane_nrows_short_row_near_stream_capacity(c_parser_only):
856856

857857

858858
@pytest.mark.parametrize("kwargs", [{}, {"dtype_backend": "pyarrow"}])
859-
def test_embedded_nul_byte_roundtrip(c_parser_only, kwargs):
860-
# GH#66277: the pyarrow string fast path computed token lengths with
859+
@pytest.mark.parametrize("prefix_len", [1, 200])
860+
def test_embedded_nul_byte_roundtrip(c_parser_only, kwargs, prefix_len):
861+
# GH#19886: the pyarrow string fast path computed token lengths with
861862
# strlen, so a quoted field with an embedded NUL byte was truncated at the
862-
# NUL instead of matching the object path
863+
# NUL. Column "b" is the last token in the stream, so its length comes
864+
# from the stream end rather than from the next token's start.
863865
pytest.importorskip("pyarrow")
864866
parser = c_parser_only
865-
result = parser.read_csv(BytesIO(b'a\n"x\x00y"\n'), **kwargs)
866-
expected = parser.read_csv(BytesIO(b'a\n"x\x00y"\n'), dtype=object)
867-
assert result["a"][0] == "x\x00y"
868-
assert expected["a"][0] == "x\x00y"
867+
value = b"x" * prefix_len + b"\x00y"
868+
data = b'a,b\n"' + value + b'","' + value + b'"'
869+
result = parser.read_csv(BytesIO(data), **kwargs)
870+
# engine="python", not dtype=object: the C object path interns words by
871+
# their NUL-terminated prefix, so it is not a sound reference for NUL data
872+
expected = read_csv(BytesIO(data), engine="python")
873+
assert result["a"][0] == value.decode()
874+
assert result["b"][0] == value.decode()
875+
assert expected["a"][0] == value.decode()
876+
assert expected["b"][0] == value.decode()

0 commit comments

Comments
 (0)