Repository navigation
Conversation
|
🚫 PR 合入质量检查未通过,PR 已被禁止合入。 未通过项:
请按上述提示整改后,重新推送(push)或评论本 PR 即可触发再次检查。 |
|
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Python | Oct 16, 2026 8:43a.m. | Review ↗ | |
| Shell | Oct 16, 2026 8:43a.m. | Review ↗ | |
| Secrets | Oct 16, 2026 8:43a.m. | Review ↗ | |
| Docker | Oct 16, 2026 8:43a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
|
🚫 PR 合入质量检查未通过,PR 已被禁止合入。 未通过项:
请按上述提示整改后,重新推送(push)或评论本 PR 即可触发再次检查。 |
|
🚫 PR 合入质量检查未通过,PR 已被禁止合入。 未通过项:
请按上述提示整改后,重新推送(push)或评论本 PR 即可触发再次检查。 |
|
🚫 PR 合入质量检查未通过,PR 已被禁止合入。 未通过项:
请按上述提示整改后,重新推送(push)或评论本 PR 即可触发再次检查。 |
|
🚫 PR 合入质量检查未通过,PR 已被禁止合入。 未通过项:
请按上述提示整改后,重新推送(push)或评论本 PR 即可触发再次检查。 |
|
🚫 PR 合入质量检查未通过,PR 已被禁止合入。 未通过项:
请按上述提示整改后,重新推送(push)或评论本 PR 即可触发再次检查。 |
|
🚫 PR 合入质量检查未通过,PR 已被禁止合入。 未通过项:
请按上述提示整改后,重新推送(push)或评论本 PR 即可触发再次检查。 |
SJTUyh
left a comment
There was a problem hiding this comment.
[review] 性能改造本身在“硬约束”上是安全的:solve_prefix_lengths 仍先算可达上下界、按 Block 取最近目标,并用与 Manifest 相同的模拟器校验最终命中 token 精确(_validated_prefix_theory),新增 scalable 路径的构造数学上也自洽(lane 内 uppers=[0]+caps[:-1]、prefix_i=h[i+1] 恰好复现 h,首个 lane 请求 hit=0),前缀长度仍是 block 对齐且不超过 length-minimum_non_shared,结果确定可复现。但确实存在多处“功能/语义层面的不兼容或退化”:(1) inspect 由“临时构造数据”改为纯配置级规划——不再加载/校验 tokenizer 与语料(tokenizer_loader 形参保留但被忽略),theoretical_hit_rate 改为计划级数值(与 effective_target_hit_rate 相同),语料缺失、tokenizer 不可加载、round-trip 失败只会在 prepare 才暴露;(2) 大规模场景的 shared_prefix_tokens 与工件内容按设计改变,且当 lane 内长度/容量不单调(如 global_shuffle / within_group_shuffle 排序)时 scalable 路径返回 None 并直接落到旧的 exact_lane_fallback,beam 因估算超阈值被跳过,平滑优化整体失效;(3) 新增的全局单调性判定会让多 lane cold 场景恒为 BEST_EFFORT(PR 自述的 1818 请求场景输出即为 BEST_EFFORT + 两条 issues),并连带把已有场景 analysis.json 的 validation.status 从 PASS 变成 PASS_WITH_WARNING;(4) 文档只补充了定性的“近线性/分 lane 构造”,没有给出任何规模—耗时规格(本次未能核实文档其他章节是否已有实测数据,也未能取到 pipeline.py 尾部完整源码,inspect 的其余改动是依据 diff 判断的)。
| "target_convergent", | ||
| ) | ||
|
|
||
| estimated_transitions = 256 * sum(cap + 1 for cap in caps) |
There was a problem hiding this comment.
[review] 算法选择用一个与实现重复的魔数决定是否完全放弃 beam:estimated_transitions = 256 * sum(cap + 1 for cap in caps)(256 与 _target_convergent_cold_prefixes 的 beam_width 默认值重复),超过 2_000_000 就只走 scalable 路径。问题是 scalable 路径要求每个 lane 的 cap/输入长度单调不减(_cold_lane_layout 第 743-746 行),否则返回 None,随后 solve_prefix_lengths 会直接落到 exact_lane_fallback(第 1201-1207 行)——也就是本 PR 想淘汰的“前缀集中在前若干条再回填”的旧构造;而对 order.strategy: global_shuffle / within_group_shuffle(或 group 长度 override 让 lane 内长度抖动)的大规模配置,lane 通常不单调,因此大规模不规则场景既拿不到 scalable 平滑解,也不会再尝试 beam(文档只写了“小型不规则场景保留受限 beam search”,未覆盖这一类)。建议:scalable 失败时用更小 beam 宽度重试一次(或先按 lane 内长度排序再走 scalable),把 256/2_000_000 改为由 beam_width 派生的常量,并把回退原因(lane 非单调/容量不足/状态爆炸)写入日志与 trajectory.issues,便于用户区分“真的不可平滑”与“被阈值跳过”。
| mode: str, | ||
| ) -> list[str]: | ||
| """Collect global and per-lane trajectory diagnostics.""" | ||
| issues = _rate_trajectory_issues("global", rows) |
There was a problem hiding this comment.
[review] _prefix_trajectory_issues 对全局累计命中率无条件做单调性/超调判定(第 1006-1009 行),但 cold 多 lane 场景中每条 lane 的首条请求按定义必然 miss(hit=0),只要请求顺序让多个 lane 交错,全局累计命中率就会出现回落、“超过最终值”的超调,于是 trajectory.status 恒为 BEST_EFFORT:PR 描述里给出的 1818 请求 / 6 group / dp_size=1 场景输出正是 status=BEST_EFFORT,issues 为 global cumulative hit rate is not monotonic 与 ... overshoots its final rate。这会带来两个后果:a) _prepare_validation 会因此恒定追加 TRAJECTORY_BEST_EFFORT,把已有(本来正常)场景的 analysis.json 的 validation.status 从 PASS 变成 PASS_WITH_WARNING,新告警失去区分度;b) 文档声称“大规模 cold 场景累计命中率平缓上升并精确落到有效目标”,与实测输出不一致。建议把“每条 lane 首条必然 miss”的可达下界计入基线(或只按 lane / DP rank 内部判定单调性,全局只判定超调),并同步修改文档措辞。
| tuple[tuple[int, ...], int], | ||
| tuple[tuple[float, float, float, float], tuple[int, ...]], | ||
| ] = {(initial_watermarks, 0): ((0.0, 0.0, 0.0, 0.0), ())} | ||
| future_capacity = _ColdFutureCapacity(caps, lane_requests) |
There was a problem hiding this comment.
[review] 这里每轮求解都会新建 _ColdFutureCapacity,但它的 lane_units 是用 @lru_cache(maxsize=65_536) 直接装饰的实例方法(定义在模块级类里):该缓存挂在函数对象上、键里包含 self,因此会把每个实例连同其持有的 caps/lane_requests 列表长期留在进程内(最多 65536 条,批量/长驻调用时表现为内存滞留)。旧实现是把 @lru_cache 加在函数内部创建的闭包上,调用返回后即可回收。建议改为实例级普通 dict 缓存,或在 total_units 内部用局部闭包 + lru_cache,使缓存随求解结束释放。
| - 理论值减目标值的带符号偏差和绝对偏差。 | ||
|
|
||
| 目标高于 `reachable_max` 或低于 `reachable_min` 时,求解器直接选择对应边界解;目标位于区间内时,按 Block 单位选择最近可达命中量。最终 hit token 精度是硬约束,在此基础上再优化请求过程:warmup 按累计输入比例均衡分配前缀;cold 按 `(Prefix Group, DP rank)` 独立水位搜索,依次最小化累计命中率的最大超调、总超调和回落,再尽快贴近目标。因此不会再把大部分前缀集中到前几条、最后用零/短前缀回调。 | ||
| 目标高于 `reachable_max` 或低于 `reachable_min` 时,求解器直接选择对应边界解;目标位于区间内时,按 Block 单位选择最近可达命中量。最终 hit token 精度是硬约束,在此基础上再优化请求过程:warmup 按累计输入比例均衡分配前缀;cold 的小型、不规则场景使用受限 beam search,大型且 lane 内长度单调的场景按 `(Prefix Group, DP rank)` 分解,先按容量和输入 token 分配目标,再在线性规模内构造逐步增长的水位。后者不再枚举每条请求的全部前缀候选,累计命中率平缓上升到目标,不会先明显超过目标再用短前缀回调。 |
There was a problem hiding this comment.
[review] 这段新增说明只给出了定性描述(“按 (Prefix Group, DP rank) 分 lane 近线性构造、不再枚举每条请求的全部前缀候选”),没有任何性能规格:多大规模(请求数 × 输入长度)下 inspect/prepare 分别需要多少时间、多少内存、走哪条路径。既然本 PR 的目标就是“大规格数据集生成”,建议补充一张实测规格表(可直接用 PR 描述里的 1818 请求、平均约 86k 输入 token、block_size=128、6 个 group 的场景,给出 inspect 与 prepare 的耗时和机器规格),并说明 estimated_transitions > 2_000_000 时会从 beam 切换到 scalable 构造、超过该规模后耗时曲线是否仍近线性;docs/source_en 与 docs/source_zh_cn 中同一段也建议补充。另外该段的“累计命中率平缓上升并精确落到有效目标”与该场景实测的 BEST_EFFORT(见 generation.py 的轨迹判定)不一致,建议按实现修正措辞。
| - [Prefix Cache plugin README](../../../plugins/prefix_cache/README_en.md) | ||
| - [Complete Scenario field reference](../../../plugins/prefix_cache/config_examples/scenario.example_en.md) | ||
| - [Multimodal benchmark MULTIMODAL.md](../../../plugins/prefix_cache/MULTIMODAL.md) | ||
| - [Prefix Cache plugin README](https://github.com/AISBench/benchmark/blob/master/plugins/prefix_cache/README_en.md) |
There was a problem hiding this comment.
[review] 文档内链由相对路径改为指向 https://github.com/AISBench/benchmark/blob/master/... 的绝对链接(zh_cn 文档与 README 同样改动):这样任何分支/发布版本的文档都会跳到 master 的最新内容,离线阅读或内网构建时也无法解析。如果原相对链接只是构建期告警,建议保留相对链接,或改用不带硬编码分支的仓库地址(至少在正文中说明链接指向 master)。
| self.assertEqual(result.strategy, "scalable_lane_convergent") | ||
| self.assertEqual(result.trajectory_status, "PASS") | ||
| self.assertFalse(result.trajectory_issues) | ||
| self.assertLess(elapsed, 5.0) |
There was a problem hiding this comment.
[review] 把墙钟断言写进单元测试容易在共享 CI / 容器 CPU 抖动时误报(self.assertLess(elapsed, 5.0)),同时 self.assertEqual(result.strategy, "scalable_lane_convergent") 与内部阈值/beam 宽度强耦合,后续微调 estimated_transitions 常数会让功能测试连带失败。建议把耗时断言放宽或拆成单独的 perf smoke(可加标记/重试),策略断言改为断言行为属性(命中 token 精确、累计命中率不超调、与 simulate_theory 一致),而不是断言内部策略字符串。
|
🚫 PR 合入质量检查未通过,PR 已被禁止合入。 未通过项:
请按上述提示整改后,重新推送(push)或评论本 PR 即可触发再次检查。 |
Thanks for your contribution; we appreciate it a lot. The following instructions will make your pull request healthier and help you get feedback more easily. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.
感谢您的贡献,我们非常重视。以下说明将使您的拉取请求更健康,更易于获得反馈。如果您不理解某些项目,请不要担心,只需提交拉取请求并从维护人员那里寻求帮助即可。
PR Type / PR类型
Related Issue | 关联 Issue
Fixes #(issue ID / issue 编号) / Relates to #(issue ID / issue 编号)
🔍 Motivation / 变更动机
Please describe the motivation of this PR and the goal you want to achieve through this PR.
请描述您的拉取请求的动机和您希望通过此拉取请求实现的目标。
prefix cache插件生成大规格数据集时,复杂度较高:
原来复杂度较高,为NNB*C
N:请求数量,例如当前场景是 1818。
B:每轮保留的 beam 状态数量,默认最多 256。
C:每条请求可选择的前缀 block 数,近似为平均 cap + 1
新增代码
大规模场景主要包含:
O(N log N)
另外降低函数复杂度,保证代码具有可维护性。
📝 Modification / 修改内容
Please briefly describe what modification is made in this PR.
请简要描述此拉取请求中进行的修改。
代码变更文档设计:
https://github.com/jschen069/benchmark/wiki/Prefix-Cache-%E4%BC%98%E5%8C%96%E4%BB%A3%E7%A0%81%E8%AF%B4%E6%98%8E
📐 Associated Test Results / 关联测试结果
Please provide links to the related test results, such as CI pipelines, test reports, etc.
请提供相关测试结果的链接,例如 CI 管道、测试报告等。
跑大规格数据集生成
scenario.json配置
执行结果:
gsm8k 生成固定数据集测试,
Does the modification introduce changes that break the backward compatibility of the downstream repositories? If so, please describe how it breaks the compatibility and how the downstream projects should modify their code to keep compatibility with this PR.
是否引入了会破坏下游存储库向后兼容性的更改?如果是,请描述它如何破坏兼容性,以及下游项目应该如何修改其代码以保持与此 PR 的兼容性。
If the modification introduces performance degradation, please describe the impact of the performance degradation and the expected performance improvement.
如果引入了性能下降,请描述性能下降的影响和预期的性能改进。
🌟 Use cases (Optional) / 使用案例(可选)
If this PR introduces a new feature, it is better to list some use cases here and update the documentation.
如果此拉取请求引入了新功能,最好在此处列出一些用例并更新文档。
✅ Checklist / 检查列表
Before PR:
After PR:
👥 Collaboration Info / 协作信息
🌟 Useful CI Command / 实用的CI命令
/gemini review/gemini summary/gemini help/readthedocs build/pr_check