Add memory-aware concurrent indexing for large repositories - #1925
Add memory-aware concurrent indexing for large repositories#1925zhiyuzhang001-a11y wants to merge 7 commits into
Conversation
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
Signed-off-by: Zhiyu <zhiyuzhang001@gmail.com>
Signed-off-by: Zhiyu <zhiyuzhang001@gmail.com>
Signed-off-by: Zhiyu <zhiyuzhang001@gmail.com>
f7032c5 to
1e15c63
Compare
Signed-off-by: Zhiyu <zhiyuzhang001@gmail.com>
Signed-off-by: Zhiyu <zhiyuzhang001@gmail.com>
Signed-off-by: Zhiyu <zhiyuzhang001@gmail.com>
Signed-off-by: Zhiyu <zhiyuzhang001@gmail.com>
|
Read in full and routed for a maintainer fit decision. Since the only thing on this thread so far is the automated acknowledgement, here is honestly where it stands rather than silence. CI is green and the validation you supplied is unusually thorough for a first contribution — ASan/UBSan focused suites, the full local suite, and named downstream results. That is not what is holding it. What needs a maintainer ruling is that this is three separable changes in one PR, and one of them is a one-way door:
Each of those is reviewable on its own evidence; together, a reservation about any one blocks all three. That is why the split matters and it is not a formality — but I am not going to ask you to do the splitting work before the direction call on (3) is made, because the answer changes what the split should look like. One thing that would genuinely help the decision, if you want to write it while this is queued: what does the scheduler do when the budget is wrong — too low, or too high on a machine it mis-measures? Failing open, failing closed, and degrading to sequential are three different products, and which one it is matters more than the admission arithmetic. I will come back with the ruling rather than leaving this to age. |
|
The maintainer ruling is in. Short version: please split this into three PRs, and the third one needs to work on all three platforms before it can be reviewed. Split into three
What (3) needs first: cross-platform measurementThe admission logic rests on DIR *directory = opendir("/proc");
if (!directory) {
return false;
}…then a walk of
Failing open is the right choice over failing closed — nobody wants indexing blocked because a probe is unavailable. The problem is that the result is a scheduler which ships its code, its five environment knobs and its maintenance cost to all three platforms while protecting only one, and does so invisibly: there is no signal at runtime that the memory half is inert. So (3) is reviewable once the RSS probe either works natively on macOS ( To be honest with you about scope: that is a substantial piece of platform work, and it is entirely reasonable to decide it is more than you want to take on right now. Splitting (1) and (2) out means neither is held hostage to that decision. One design question for whenever (3) comes back
Credit where it is dueThe validation you supplied is well beyond what a first contribution usually carries: ASan/UBSan focused suites, the full local suite, and named downstream results rather than "tests pass". The None of the above is a rejection of the idea. It is a request to let the two uncontroversial pieces land on their own evidence while the third gets the platform work it needs. |
Summary
Motivation
This enables multiple project-scoped MCP clients to share the Provider safely. Small daily repositories can run concurrently; large jobs are admitted according to measured source bytes and memory budget instead of starting without a global bound.
Validation
maingit diff --checkpassesNo release or binary distribution is included in this PR.