GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-150942: Speed up csv.reader row building by omkar-334 · Pull Request #150995 · python/cpython

Conversation

@omkar-334

@omkar-334 omkar-334 commented Jun 6, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Use _PyList_AppendTakeRef while collecting fields in csv.reader instead of
PyList_Append followed by Py_DECREF, removing an incref/decref pair per
parsed field.

Microbenchmarks

Release build, macOS

Benchmark main this PR speedup
csv_reader_2m_fields 86.96 ms 79.23 ms 1.10x
csv_reader_400k_fields_8col 17.15 ms 16.26 ms 1.05x
csv_reader_80k_fields 3.17 ms 2.89 ms 1.10x
geomean 1.08x
Benchmark script
"""Micro-benchmark for csv.reader field-append hot path.

Generates an in-memory CSV with many fields per row, then times how long it
takes to iterate the reader fully. Repeats N trials, reports min/median/mean
so noise is visible. Writes per-trial timings to results/.
"""

from __future__ import annotations

import argparse
import csv
import io
import json
import statistics
import sys
import time
from pathlib import Path


def make_csv(rows: int, cols: int) -> str:
    buf = io.StringIO()
    w = csv.writer(buf)
    for r in range(rows):
        w.writerow([f"v{r}_{c}" for c in range(cols)])
    return buf.getvalue()


def time_reader(data: str) -> float:
    t0 = time.perf_counter_ns()
    for _ in csv.reader(io.StringIO(data)):
        pass
    return (time.perf_counter_ns() - t0) / 1e9


def main() -> int:
    ap = argparse.ArgumentParser()
    ap.add_argument("--rows", type=int, default=20_000)
    ap.add_argument("--cols", type=int, default=20)
    ap.add_argument("--trials", type=int, default=15)
    ap.add_argument("--warmup", type=int, default=3)
    ap.add_argument("--label", default="run")
    ap.add_argument("--outdir", default="results/gh-150942")
    args = ap.parse_args()

    data = make_csv(args.rows, args.cols)
    size_bytes = len(data.encode())
    field_count = args.rows * args.cols

    for _ in range(args.warmup):
        time_reader(data)

    trials = [time_reader(data) for _ in range(args.trials)]

    outdir = Path(args.outdir)
    outdir.mkdir(parents=True, exist_ok=True)
    summary = {
        "label": args.label,
        "python": sys.executable,
        "rows": args.rows,
        "cols": args.cols,
        "trials": args.trials,
        "warmup": args.warmup,
        "field_count": field_count,
        "input_bytes": size_bytes,
        "seconds": {
            "min": min(trials),
            "median": statistics.median(trials),
            "mean": statistics.fmean(trials),
            "stdev": statistics.pstdev(trials),
            "all": trials,
        },
        "fields_per_sec_at_min": field_count / min(trials),
    }
    out = outdir / f"bench_{args.label}.json"
    out.write_text(json.dumps(summary, indent=2))

    s = summary["seconds"]
    print(
        f"{args.label}: min={s['min']*1000:.2f}ms median={s['median']*1000:.2f}ms "
        f"mean={s['mean']*1000:.2f}ms stdev={s['stdev']*1000:.2f}ms "
        f"fields/s@min={summary['fields_per_sec_at_min']:.2e}"
    )
    print(f"wrote {out}")
    return 0


if __name__ == "__main__":
    raise SystemExit(main())

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@omkar-334
pythongh-150942: Speed up csv.reader row building
7945e61
@bedevere-app bedevere-app Bot added the awaiting review label Jun 6, 2026
@bedevere-app bedevere-app Bot mentioned this pull request Jun 6, 2026
Improve performance by using reference stealing methods #150942
Open
omkar-334 added 3 commits June 6, 2026 07:23
@omkar-334
Merge branch 'main' into pythongh-150942-csv-appendtakeref
1c00c00
@omkar-334
add patch contributor name
ed09dac
@omkar-334
Merge branch 'main' into pythongh-150942-csv-appendtakeref
b29bdae
eendebakpt
eendebakpt reviewed Jun 6, 2026
View reviewed changes
Comment thread Modules/_csv.c
}
if (PyList_Append(self->fields, field) < 0) {
Py_DECREF(field);
if (_PyList_AppendTakeRef((PyListObject *)self->fields, field) < 0) {

@eendebakpt eendebakpt Jun 6, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The list used here is self->fields. Can we guarantee that it is not mutated by concurrent threads?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@omkar-334 omkar-334 Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

when parse_save_field is running, self->fields is only reachable through self, and self is locked by `Reader_iternext's critical section. i dont think any other thread can mutate the list.

static PyObject *
Reader_iternext(PyObject *op)
{
    PyObject *result;
    Py_BEGIN_CRITICAL_SECTION(op);
    result = Reader_iternext_lock_held(op);
    Py_END_CRITICAL_SECTION();
    return result;
}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@omkar-334 omkar-334 Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

what do you think?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@omkar-334 omkar-334 Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

cc @eendebakpt , just a nudge

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@eendebakpt eendebakpt Jun 19, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The object is locked and self->fields is not accessible via other paths so this is indeed safe.

If redesigning I would make probably make fields (and maybe some other fields) a local variable inside Reader_iternext_lock_held (only minor thing to take care of: the fields currently acts as a guard for re-entrant calls). This is out of scope for the PR though.

I would drop the new entry (or at least omit the implementation details).

@omkar-334 This needs to be reviewed by a core dev, might it might take some time.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

omkar-334 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@eendebakpt eendebakpt mentioned this pull request Aug 17, 2026
gh-150942: Improve performance of csv reader by using _PyList_AppendT… #155957
Closed
StanFromIreland
StanFromIreland reviewed Aug 17, 2026
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2026-06-05-12-00-00.gh-issue-150942.Hkdh87.rst Outdated
@@ -0,0 +1,3 @@
Speed up :func:`csv.reader` by appending parsed fields to the output row
without an extra reference-count round-trip (using the internal
reference-stealing list append helper). Patch by Omkar Kabde.

@StanFromIreland StanFromIreland Aug 17, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This news entry has too many implementation details, all of which have no impact on the end user. Please remove them.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@omkar-334 omkar-334 Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@StanFromIreland I've now simplifed the news entry. Or did you mean I should remove it entirely?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
omkar-334 added 2 commits August 19, 2026 10:35
@omkar-334
simpify news entry
9c6be23
@omkar-334
Merge branch 'main' into pythongh-150942-csv-appendtakeref
d695bf9
@eendebakpt eendebakpt mentioned this pull request Sep 1, 2026
gh-156748 : Performance - Use takeref append in _csv #156749
Closed
serhiy-storchaka
serhiy-storchaka reviewed Sep 11, 2026
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I only get difference 1-2% in normal cases (20% for empty fields, which are not common). The optimization is itself low-cost (if we not decide to use stable ABI only in this module) and low-risk, but it does not deserve a NEWS entry.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

Reviewers

@eendebakpt eendebakpt eendebakpt left review comments
@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments
@StanFromIreland StanFromIreland StanFromIreland left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@omkar-334 @eendebakpt @serhiy-storchaka @StanFromIreland
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.