dolthub / dolthub/doltlite

doltliteParseRecordStrict memsets 16 KB per record parse, on the per-row path of every VC read surface

Closed Beginner friendly
#2,918 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
268
Forks
18
Avg merge
2h 27m
Merged PRs (30d)
447

Description

Found in a full-repo review at `0ba280f06f`.

`doltliteParseRecordStrict` zeroes a 16 KB struct on every call, on the per-row path of every version-control read surface.

`src/doltlite_record.c:529`:

```c
memset(pInfo, 0, sizeof(*pInfo));
```

`DoltliteRecordInfo` (`src/prolly_record.h:83-88`) is `4 + 2 × DOLTLITE_MAX_RECORD_FIELDS × 4` bytes, and `DOLTLITE_MAX_RECORD_FIELDS` is `SQLITE_MAX_COLUMN` (2000), so **16,004 bytes** — regardless of how many fields the record actually has. Only `[0..nField)` is ever read, and every reader guards on `nField`.

This is not an incidental path. `diffRecordsEqualFieldwise` (`src/prolly_diff.c:147-210`) declares *two* of these and parses both per row-pair, and it is the equality test behind `prollyValuesEqual`, which diff, merge, blame and status all use per row.

## Measured

Replacing the memset with `pInfo->nField = 0;` in a scratch build, 1M-row table with every row changed, median of 3:

| workload | master | one-line patch | speedup |
|---|---|---|---|
| `count(*) FROM dolt_diff_t` | 1.461 s | 1.073 s | **1.36x** |
| `count(*) FROM dolt_blame_t` | 1.075 s | 0.693 s | **1.55x** |
| `dolt_diff_stat('HEAD~1','HEAD')` | 0.025 s | 0.025 s | 1.00x |
| `dolt_diff_summary('HEAD~1','HEAD')` | 0.026 s | 0.026 s | 1.00x |

Results identical in both builds.

## Fix

Set `nField = 0` instead of zeroing the whole struct — the readers already bound themselves by `nField`, so nothing else depends on the zeroing. Worth doing alongside: shrink `DoltliteRecordInfo` to a small inline array with heap spill for wide records. It is a 16 KB stack object, so it is also the direct cause of most of the oversized frames in the wasm stack report.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start at doltliteParseRecordStrict in src/doltlite_record.c:529 and inspect DoltliteRecordInfo in src/prolly_record.h:83-88, then trace its two callers in diffRecordsEqualFieldwise at src/prolly_diff.c:147-210. Validate the change against diff and blame workloads, checking that results remain identical and the per-row performance improves without affecting the summary operations.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, sqlite
Domain
databases, performance
Issue type
Refactor
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
82/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.