dpl: updateHistCost() runtime improvement#10979
Conversation
replace std::unordered_set with std::vector for pixel history-cost dedup Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request optimizes pixel deduplication in updateHistoryCosts by replacing the std::unordered_set with a stamp-based approach using a std::vector<uint32_t> and a generation counter. Feedback suggests defensively handling potential integer overflow of the generation counter to avoid state conflicts, and using size_t instead of int for the pixel index calculation to prevent signed integer overflow on large grids.
gudeh
changed the title
Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>
|
@codex review |
| ++hist_gen_; | ||
| if (hist_gen_ == 0) { |
There was a problem hiding this comment.
How will it be zero after you just incremented it?
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7e6414663
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // legalize() can run again on this same object, and grid_w_/grid_h_ may be | ||
| // different next time. | ||
| hist_seen_stamp_.assign(static_cast<size_t>(grid_w_) * grid_h_, 0); |
There was a problem hiding this comment.
This commit has no Signed-off-by: trailer (git interpret-trailers --parse returns no trailers), despite its message claiming DCO compliance. Add the author sign-off so the commit satisfies the repository's mandatory DCO requirement and can pass the corresponding merge check.
AGENTS.md reference: AGENTS.md:L18-L18
Useful? React with 👍 / 👎.
2 participants
Summary
Replace
std::unodered_settostd::vectorfor container to check already visited pixels atNegotiationLegalizer::updateHistoryCosts().While investigating with VTune I realized our current bottleneck is with updateHistoryCosts(), with system time at
std::unodered_set.With this change we get the following situation:
Type of Change
Impact
For a private design with 413K instances we go from 48sec to 34sec to execute negotiation alone.
Verification
./etc/Build.sh).Related Issues
We had other recent runtime improvements for negotiation: #10929, #10936, #10949. All these improvements compound, with each reducing runtime on its own.