tamnd / tamnd/firepanda

A fourth word on Buffer costs half a megabyte of code

Open
#811 0 comments 0 reactions 0 assignees View on GitHub
area/kernel performance
Dominant language
Mojo
Stars
1
Forks
0
Avg merge
1h 31m
Merged PRs (30d)
640

Description

Buffer grew a fourth field so a window could remember where in its parent it starts, and the extension's text went from 7,426,521 bytes to 7,943,001 on x86-64. That is 516,480 bytes for one word, and it pushed the packaged extension past the size budget in python/tests/test_extension.py, which was raised from ten mebibytes to eleven to let the window work land.

Where it comes from was measured rather than guessed, by building the same commit three ways on one machine with everything else held still.

Building main with only firepanda/buffer/buffer.mojo taken from the window branch gives 10,694,920 bytes across the four files and 7,943,001 bytes of text, which is what the whole branch gives, to the byte. So none of the growth is the new window methods on Bitmap, ColumnData, StringArray and AnyArray, and none of it is the scan that calls them.

Building main with a single unused Int field added to Buffer, set to zero in each of the three constructors and read by nothing, gives 7,888,313 bytes of text. That is 461,792 bytes, or 89 per cent of the whole thing, for a field the program never touches. The offset arithmetic in unsafe_ptr and its four neighbours is the remaining 54,688.

So the cost is Buffer going from three machine words to four. Every struct that embeds a Buffer grows with it, ColumnData holds two of them, and an AnyArray holds a ColumnData, so the layout change reaches most of the engine. Twenty four bytes to thirty two is the sort of step where a compiler stops passing a value in registers and starts passing it in memory, which would explain a broad few per cent rather than a few large functions.

What to try, in the order they look worth trying.

Derive the capacity instead of storing it. For every buffer the constructors make, capacity is the size rounded up to 64 with a floor of 64. For a window it is the same: an interior window has a size that is already a multiple of 64, and a window that runs to its parent's end has round_up(parent_size) - at, which equals round_up(size) because at is a multiple of 64. The one place the two part company is the shrink that lowers the size and leaves the capacity where it was, so that path has to be looked at before anything else. If capacity goes, offset takes its slot and Buffer stays three words.

Narrow the fields. Size, capacity and offset in UInt32 is twenty bytes and rounds to twenty four, but it caps a buffer at four gigabytes, and a string payload on a large column can pass that. Only worth it if the first idea does not work.

Confirm the register threshold rather than assuming it. If thirty two bytes is not where the calling convention changes, the diagnosis above is wrong and the growth is something else that a layout change happens to trigger.

Whatever the fix, the check is the same three way build: the extension's text back at or near 7,426,521, and the budget lowered again in the same commit rather than left where the window work put it.

Contributor guide

Open the contributing guide

Research direction

Start with firepanda/buffer/buffer.mojo, tracing the shrink path and how Buffer fields are initialized and used. Run the three-way build described in the issue and inspect python/tests/test_extension.py for the size budget. Done means preserving window behavior while bringing extension text near 7,426,521 bytes and lowering the budget in the same commit.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
build-system, data-engineering, performance
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.