[Backport 1.0] perf(hgraph): reduce visited list memory#2452
Conversation
Signed-off-by: jac0626 <jac0626@users.noreply.github.com> Assisted-by: Codex:GPT-5
Signed-off-by: jc543239 <jc543239@antgroup.com> Assisted-by: Codex:GPT-5
Signed-off-by: jc543239 <jc543239@antgroup.com> Assisted-by: Codex:GPT-5
|
/label S-waiting-on-review |
There was a problem hiding this comment.
Code Review
This pull request refactors the VisitedList class to use a bit-packed representation, grouping 64 elements into a single uint64_t word with associated uint16_t tags. This change significantly optimizes memory usage. Additionally, comprehensive unit tests have been added, including boundary, tag overflow, and randomized differential tests. The review feedback highlights a potential null-pointer dereference if the allocator fails to allocate memory, which should be addressed to prevent undefined behavior.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| auto* buffer = static_cast<uint8_t*>(allocator_->Allocate(words_bytes + tags_bytes)); | ||
| this->words_ = reinterpret_cast<WordType*>(buffer); | ||
| this->tags_ = reinterpret_cast<TagType*>(buffer + words_bytes); | ||
| memset(tags_, 0, tags_bytes); |
There was a problem hiding this comment.
If allocator_->Allocate returns nullptr (e.g., due to an out-of-memory condition), performing pointer arithmetic buffer + words_bytes and calling memset on tags_ will result in undefined behavior and a segmentation fault.
Consider adding a check or assertion to ensure buffer is not nullptr before using it, or verify if the allocator is guaranteed to throw on failure.
Summary
Backports #2449 to the
1.0branch.Validation
make fmtwith clang-format 15git diff --checksrc/utils/visited_list.cppandsrc/utils/visited_list_test.cppwith the CMake-generated compilation commandsmake test CASE='[ut][VisitedList]'was blocked before project compilation by external dependency download timeouts; CI will run the targeted and repository checks with its dependency cacheBackport of #2449.