Repository navigation
Replies: 4 comments 1 reply
这是一个读放大问题——而且你的量测方式本身值得肯定1. 你指出的形态是清楚的
建议在报告里把这个复杂度写出来(例如"单次查找 2. 修法方向(按收益排序)
建议把第 1 条写成最低要求(改动最小、收益最大),第 2、3 条作为后续。 3. 请补三样(把"慢"变成可复现的数字)
4. 一条相关线索同日另有一条关于图片处理的报告( 5. 关于可复现性你这条天然可复现(同一索引、反复解析同一批图片即可)。建议写成一个可跑的判据,例如:"在含 N 条记录的索引上连续 一条边界我没有读 |
|
Source-verified on 1. The chain: one full read and one full validation per image, per HTTP attempt
2. Where the time actually goesPublished
Parse + per-record validation ≈ 86%; the file read is the minority. A cache that only avoids the read (the OS page cache already absorbs much of it) recovers little — the parse/validate result is what has to be reused. Validation is 3. Read amplification is exactly 1:1, deterministically50 4. Your identity check has a stronger form availableEvery local write goes through 5. Why this is a plugin-shaped gap (and its boundary)There is no cordis service to shadow here: 6. Two notes on your patch
Boundary: static reading of |
|
A plugin for this gap: The analysis in this thread already says what the change should be — memoize the parse, leave everything else alone — so this is that change packaged rather than another proposal. I built and published it.
Mounting it, through the bundle-patch convention (a direct - insert:
- id: upload-index-cache
name: '@argszero/dsh-upload-index-cache'What it does. It memoizes It does not touch the parser, the expiry rule, the lock, the atomic write, the v3 format or Why The seam, since Verification. 52 tests, including a control arm that runs the same sequence with the plugin absent and watches the second lookup fail with Peer range: One real difference, stated rather than left to be discovered: callers of the memoized |
|
Gentle ping @CreatixChu, since you recently worked on the DeepSeek Files upload index. When you have a moment, would a small parsed-index cache be something the team would consider in the native adapter, or would you prefer this to remain a community plugin for now? The report now includes a keyless reproduction, per-stage timings, and an independently reproduced diagnosis from argszero above. Thank you to argszero for investigating and providing a plugin option as well. Our reference implementation is in fork PR #1. In a local synthetic benchmark with roughly 500 retained mappings, the lookup batch went from 567.50 ms to 21.49 ms; this is local index time, not an end-to-end model or billing claim. The latest follow-up adds exact index byte counts and strengthens the same-size/restored-mtime regression. All 37 focused index tests, documentation checks, lint, and the normal pre-push typecheck passed. We understand that external PRs are not currently accepted; the fork is just a reviewable reference if useful. Happy to adjust the scope to the team's preferred approach. No urgency, and thanks for your time. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Summary
While profiling long sessions with hundreds of retained images, I found that resolving images which were already uploaded still spent a surprising amount of time in the local upload index.
DeepSeekUploadIndex.get()reads and parses the entirefiles-v3.jsonfile, validates all its records, and scans the records for each lookup. Resolving 500 retained image mappings therefore reads and parses the same index roughly 500 times before sending the next model request. Adding one new image between requests keeps this overhead on the request path.I'm posting here because the contribution guide directs bug reports to Discussions and the repository currently has Issues and external PRs disabled. I have prepared a small implementation in a fork for review; it does not change model settings or prompts.
Reproduction
The contribution includes a keyless benchmark at
packages/llm/llm-deepseek/benchmarks/upload-index.ts. It uses synthetic upload records, so it needs no images, credentials, GPU, or API calls.From the contribution checkout, after
pnpm install --frozen-lockfile:Each batch commits one new mapping, then looks up every mapping and checks the returned file IDs. There are two warmup batches followed by five measured batches. Timings include the first lookup after each commit, but exclude the commit itself.
Current behavior
Here are the results from the same machine, using the same benchmark for both versions:
These are local lookup timings, not end-to-end model latency. The 500-record case took about 97% less lookup time. I am not claiming a provider token-cache or billing improvement from this patch; it does not change the request sent to the model.
Expected behavior
An unchanged index should be parsed once and reused for subsequent image lookups. A new upload or another process changing the index must still become visible, and expiry checks must still use the current time and refresh margin.
Environment
@deepseek-ai/dsh-llm-deepseek:0.2.1-alpha.1.5badb15009ae1756c3afe0ae0cef1faafc290ccc.Proposed fix
Keep one parsed document and a lookup map per index instance. Check the file identity, size, and nanosecond modification/change times before reuse, and check again before retaining a newly read document. Invalidate the cache before and after local writes so an overlapping read cannot leave an older generation installed after the write finishes.
The patch retains the existing parser, locks, atomic writes, expiry rules, exact-generation removal, and v3 file format. Returned records are copied so a caller cannot modify the cached records. Only index files up to 8 MiB are eligible for retained parsing; larger files keep the uncached behavior. That ceiling measures serialized file size, not JavaScript heap usage.
The focused index and Files tests pass (73 tests), with 100% statement, branch, function and line coverage for the changed index module. Running the two new read-reuse regressions against the original implementation fails as expected: they observe six and three reads where the patched implementation reads once.
I would be happy to adjust the approach if there is a preferred way to cache this index upstream.
Implementation: xuhao1@adc30f3
Review PR (in my fork; not an upstream PR): xuhao1#1
All reactions