airborne12 opened a new pull request, #69728:
URL: https://github.com/apache/doris/pull/69728
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: `SniiBkdIndexColumnWriter`, the numeric, date and IP index
writer, installed no memory reporter. Its point buffer, and its NULL docid
vector, which grows with the row count, were invisible to the process-wide SNII
build tracker (the `SniiIndexBuild` MemTracker and the
`snii_index_build_mem_consumption` gauge) that the text writer feeds.
The writer now creates a reporter in the unregistered population, since a
forced spill cannot reclaim a BKD build, and hands it to the BkdBuilder. It
charges the NULL docids through the growth routine the text writer already
used, which is extracted to `reserve_docids_for_append` next to
`MemoryReporter` so that the two writers share it. It gives the vector and its
charge back as soon as `finish()` has moved the docids into the bitmap, because
IndexBuilder keeps finished writers alive until the whole rowset is closed.
This only adds accounting. The reporter has no cap, so it cannot fail a
build, and its callback charges a plain MemTracker, not a limiter.
### Release note
The SNII index build memory metric now also counts the point buffers and
NULL rows of numeric, date and IP indexes.
### Compatibility
- No on-disk format change and no reader change: the BKD writer writes the
same bytes as before.
- The reporter only mirrors byte counts into the `SniiIndexBuild` MemTracker
and the `snii_index_build_mem_consumption` gauge through a callback. It has no
cap, so a reservation cannot fail (only an int64 overflow of the counter
could), and the callback charges a plain MemTracker, never a MemTrackerLimiter,
which would count the same bytes twice because the allocator hook already
attributes them. So a build cannot fail where it passed before.
- Nothing is exchanged between nodes, so mixed versions are not affected.
The text writer uses the extracted helper with the same behavior: the larger
capacity is charged before the allocation and the old charge is returned after
the move.
### Performance
- Only the capacity check of `reserve_docids_for_append` is inline in its
callers. The growth step is out of line (`NO_INLINE`), since it runs O(log n)
times per writer. Compiled with `-O2` and `-O3`, the text writer's
`_reserve_null_docids_for_append` goes from about 200 instructions, with a
six-register prologue on every call, to about 10 instructions with no prologue
when the capacity is enough. An earlier version of the helper, with the growth
step inline, was not inlined into its callers and cost the text writer one
extra call frame per call, about 2% in the first row of the benchmark below.
- Appending NULL docids in runs of random length, 100 million rows, best of
5 repetitions per process, interleaved, one pinned core. `plain` is the BKD
writer before this change, `before` the text writer's routine before, `after`
the shared helper:
| docids per call | plain | before | after | after / before |
|---|---|---|---|---|
| 1 to 20 | 60.3 ms | 51.8 ms | 50.5 ms | 0.976 |
| 1 to 200 | 217.9 ms | 244.7 ms | 251.0 ms | 1.026 |
| 1 to 20000 | 363.2 ms | 325.9 ms | 320.1 ms | 0.982 |
The text writer is unchanged within the noise of this machine (medians
vary by up to 7%). The BKD writer pays for the accounting only in the middle
row, 33 ms for 50 million NULL docids, about 0.7 ns each; in the other two rows
it is faster than before.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- Added SniiBkdAdapterTest.BuildMemoryIsChargedAndGivenBackAtFinish.
All BE unit tests matching *Snii* pass. With the production change reverted,
the new case fails.
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason
- Behavior changed:
- [x] No. The bytes are only reported to the build tracker, and are
returned at finish.
- [ ] Yes.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
