Skip to content

gh-156748 : Performance - Use takeref append in _csv - #156749

Closed
brittanyrey wants to merge 2 commits into
python:mainfrom
brittanyrey:b-perf-csv-append-takeref
Closed

gh-156748 : Performance - Use takeref append in _csv#156749
brittanyrey wants to merge 2 commits into
python:mainfrom
brittanyrey:b-perf-csv-append-takeref

Conversation

@brittanyrey

@brittanyrey brittanyrey commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Reduce refcount churn by consuming the reference instead in lieu of calling append and decreffing.

Benchmark base patched delta
50 fields × 200 rows 537.7 µs 508.3 µs −5.5%
quoted + embedded separators/newlines 468.9 µs 435.8 µs −7.1%
all-empty fields (50 × 200) 173.6 µs 150.0 µs −13.6%
2 fields × 200 rows 24.0 µs 23.3 µs −2.9% (noise 3.3%)
5 × 500-char fields (control) 2517.7 µs 2508.2 µs −0.4% (noise 21%)
# 50 fields x 200 rows
data = "\n".join(",".join(f"f{j}" for j in range(50)) for _ in range(200))
list(csv.reader(io.StringIO(data)))

@@ -0,0 +1 @@
Optimize refcount churn in _csv.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in the Library folder. And refcount churn is not a user-observable detail, I think we should just mention the performance optimization.

@aisk

aisk commented Sep 1, 2026

Copy link
Copy Markdown
Member

I ran the benchmarks on my machine (MacBook Air with M3 chip) and got similar results:

Benchmark base patched delta PR
50 fields × 200 rows 408.5 µs 389.9 µs −4.6% −5.5%
quoted + embedded sep/newlines 725.3 µs 707.6 µs −2.4% −7.1%
all-empty fields (50 × 200) 115.8 µs 105.5 µs −8.9% −13.6%
2 fields × 200 rows 16.5 µs 16.1 µs −2.6% −2.9%
5 × 500-char fields (control) 2288.6 µs 2282.8 µs −0.3% −0.4%

@eendebakpt

Copy link
Copy Markdown
Contributor

This is a duplicate of #150995. I will close in favor of the earlier PR.

@eendebakpt eendebakpt closed this Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants