Skip to content

perf(rst): build the export data once instead of three times - #670

Open
dexhunter wants to merge 1 commit into
jazzband:masterfrom
dexhunter:perf/rst-export-single-pass
Open

perf(rst): build the export data once instead of three times#670
dexhunter wants to merge 1 commit into
jazzband:masterfrom
dexhunter:perf/rst-export-single-pass

Conversation

@dexhunter

Copy link
Copy Markdown

What this changes

ReSTFormat.export_set reads dataset.dict three times for a single export:

  • export_set reads it to test whether the dataset is empty
  • _get_column_string_lengths reads it to measure the column widths
  • the table builder reads it again to render the rows

Each read repackages the entire dataset. When headers are set, _package also
allocates one dict per row, and every consumer in _rst.py immediately drops the
keys again by calling row.values(). Each cell also goes through to_str twice,
once when its length is measured and once when it is rendered.

This packages the dataset once with _package(dicts=False), which is what
_csv.py, _xls.py, _xlsx.py and _ods.py already use, converts each cell
once, and passes the strings down to both the width calculation and the table
builders.

Two smaller changes on the same path:

  • A cell that contains no whitespace and already fits its column comes back from
    TextWrapper unchanged, so that line is built directly. Widths of zero or less
    still go to TextWrapper, which rejects them.
  • A cell cannot contain a word longer than itself, so _max_word_len is skipped
    when the cell cannot raise the running maximum.

Numbers

Time for one Dataset.export('rst') call at stock defaults (no kwargs,
MAX_TABLE_WIDTH = 80), CPython 3.12.13, median of 7 runs with a freshly
generated dataset per run:

dataset before after change
200 rows × 5 cols 5.25 ms 2.08 ms −60.4%
1,000 × 5 25.42 ms 10.05 ms −60.5%
1,000 × 12 55.90 ms 22.40 ms −59.9%
5,000 × 5 128.91 ms 50.74 ms −60.6%

The denominator is the export('rst') call itself. _package runs 3 times per
export before this change and once after, counted by instrumenting
Dataset._package during one export. This is CPU on the rst export path. It says
nothing about the other formats or about IO.

Output is unchanged

Checked byte for byte against the current master implementation over 76 cases:
empty datasets, single cells, all-empty columns, mixed cell types (int, float,
bool, None, bytes), unicode, whitespace-padded and tab-containing cells,
wrapped and unwrapped first columns, force_grid, max_table_width from 10 to
400, and a randomized matrix of shapes and keyword combinations. All 76 produce
identical bytes.

That includes duplicate header names. dataset.dict routes each row through
dict(zip(headers, row)), so a repeated header keeps only the last column
carrying that name while the header row still shows every header. That looked
like something to fix, but it is current behaviour and changing it did not belong
in a performance patch, so this reproduces it exactly. The collapsing column
positions are resolved once instead of rebuilding a dict per row. Happy to raise
it separately if you think it is worth changing.

Registered formatters still apply, because the rows come from _package rather
than from Dataset._data. export_set_as_simple_table and
export_set_as_grid_table keep their existing signatures with the new parameter
optional, so calling them directly with just a dataset still works.

The test suite passes, 183 tests, and ruff is clean.

How I found it

Profiling export('rst') on a 2,000-row dataset put TextWrapper.wrap at 43% of
the time, the width pass at 26%, _max_word_len at 10% and to_str at 6%. I
then ran an automated optimization search over _rst.py, gated on byte-exact
output and on the repository's own test suite. The trajectory is here:

https://dashboard.weco.ai/share/3_8MTQcLFq67dmDdp-N7s5eZbhhM1mQj

The patch in this PR is a smaller hand-written version of what that search found.
Its best step was a little faster again on my benchmark harness, but it rewrote
most of the module and stopped decoding bytes subclasses, so I kept the
narrower change.

ReSTFormat read dataset.dict three times for every export: once to test
whether the dataset was empty, once to measure the column widths and once
to render the rows. Each read repackages the whole dataset, and when
headers are set it allocates a dict per row that every caller here throws
away again by calling row.values(). Every cell was also converted with
to_str twice, once to measure it and once to render it.

Package the dataset once via _package(dicts=False), which is what the csv,
xls, xlsx and ods formats already use, convert each cell once, and pass the
strings down to the width calculation and the table builders.

Two smaller changes on the same path: cells that contain no whitespace and
already fit their column are returned unchanged by TextWrapper, so build
those lines directly; and a cell can never contain a word longer than
itself, so skip the split when it cannot raise the running maximum.

Output is unchanged, including the existing behaviour where duplicate
header names collapse columns in the rendered rows.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant