Repository navigation
Conversation
qicosmos
commented
Aug 12, 2026
qicosmos
left a comment
Owner
Author
There was a problem hiding this comment.
主要问题:
-
编译错误风险:
benchmark/json_benchmark.cpp约 275 行起多处使用ScopedTimer timer(name.data(), ns);,但 diff 中ScopedTimer构造函数仍显示只接受const char*。若未新增重载会直接编译失败。 -
CMake 顺序/目标风险:
CMakeLists.txt约 136 行对json_benchmark调用set_target_properties/target_compile_options,需确认该 target 一定已创建,否则配置失败。 -
AVX2 选项安全性:约 145 行仅按
CMAKE_SYSTEM_PROCESSOR判断,不代表运行机器支持 AVX2。开启后生成二进制在非 AVX2 CPU 上会非法指令,建议文档明确或做运行时分发。 -
benchmark 公平性:DOM parse 每轮构造/销毁
document,而 reuse 版本复用容量;结果需明确区分,否则容易误导。 -
代码风格:
volatile size_t json_parse_sink用于防优化可以接受,但json_parse_sink = json_parse_sink + value不必要,建议用更标准的 benchmark 防优化方式。
qicosmos
commented
Oct 17, 2026
qicosmos
left a comment
Owner
Author
There was a problem hiding this comment.
结论:核心解析器及测试未展示,无法确认正确性与内存安全;以下仅针对可见 diff。
- 中|CMakeLists.txt:136 附近:将既有
json_benchmark强制升级到 C++20,扩大了新功能对原构建的影响。建议拆分 DOM benchmark,保留原目标的标准要求。 - 中|CMakeLists.txt:144–153:AVX2 仅按目标架构启用,未检测运行 CPU 是否支持。开启后在旧 x86 CPU 上可能非法指令崩溃;建议增加运行时分派,或明确部署限制。
- 中|benchmark/json_benchmark.cpp:275 起:各方案生命周期不一致:旧 DOM 复用对象,新 DOM 部分每轮构造析构,reuse 版本又在计时外预分配。结果不能直接归因于解析性能;应分组比较同等复用与分配策略。
- 低|新增 benchmark 函数:计时、循环、输出大量重复,缩进也不统一;建议抽取公共测试框架。
- 审查缺口:请补充
json_dom.hpp和测试 diff,重点核查截断输入、UTF-8/转义、数值溢出、嵌套深度及 view 生命周期。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
Iguana needed a reusable DOM parsing path for workloads that require dynamic JSON access while retaining competitive throughput and avoiding per-parse allocations where possible.
Performance
On the controlled fixed-core corpus run, Iguana reached 2.271 GiB/s versus simdjson at 2.466 GiB/s, a 7.81% gap. The retained continuation-path optimization improved array strings by 15.54%, object strings by 8.39%, and array numbers by 20.77%.
Validation
git diff --check