odb: speedup dbITerm::getMTerm()#9538
odb: speedup dbITerm::getMTerm()#9538openroad-ci wants to merge 1 commit intoThe-OpenROAD-Project:masterfrom
Conversation
![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 introduces a performance optimization for dbITerm::getMTerm() by caching master terminal pointers (_dbMTerm*) in the _dbInstHdr. This avoids repeated lookups through the database hierarchy, which should improve performance as intended. The changes are well-contained and correctly handle cache population during dbInstHdr creation, dynamic updates in hierarchical blocks, and deserialization. The use of the cache in getMTerm is straightforward and achieves the intended speedup. The implementation appears solid and I have no further comments.
openroad-ci
force-pushed
the
odb-getmterm-speedup
branch
from
0daa238 to
d705140
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| dbId<_dbLib> lib_; | ||
| dbId<_dbMaster> master_; | ||
| dbVector<dbId<_dbMTerm>> mterms_; | ||
| std::vector<_dbMTerm*> mterm_ptrs_; // non-persistent |
There was a problem hiding this comment.
warning: no header providing "std::vector" is directly included [misc-include-cleaner]
src/odb/src/db/dbInstHdr.h:6:
+ #include <vector>
Cache the mterms on the dbInstHeader. Intended to help sta performance. Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
openroad-ci
force-pushed
the
odb-getmterm-speedup
branch
from
d705140 to
3db5ed3
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
|
@maliberty Why not just store a dbMTerm* on the Iterm itself. I found it way faster to do that than go through the indirection of the vector. |
|
That is a lot more memory as iterms are numerous. @dsengupta0628 will be profiling this so we'll get a sense if this is enough. |
|
Even on our 3 million standard cell block it only increases memory by 434MB. Against a 78GB total ram usage it's not much. The problem with the vector is that you're taking two indirections and thrashing the cache. On most of my benchmarks i've seen MTerm take anywhere from 5-6% of the profile, I'd really make sure you're not just moving that cost around with this solution. |
|
I'd really urge you to do the comparison yourself. Just add the pointer to Iterm and load an existing ODB subtract the memory difference. |
3 participants
Cache the mterms on the dbInstHeader. Intended to help sta performance.