Skip to content

odb: cache x/y grids in dbTrackGrid#8854

Merged
maliberty merged 3 commits intoThe-OpenROAD-Project:masterfrom
gadfort:odb-cache-grids
Nov 17, 2025
Merged

odb: cache x/y grids in dbTrackGrid#8854
maliberty merged 3 commits intoThe-OpenROAD-Project:masterfrom
gadfort:odb-cache-grids

Conversation

@gadfort
Copy link
Collaborator

@gadfort gadfort commented Nov 17, 2025

No functional changes expected.

Changes:

  • caches the results of the x/y grids in dbTrackGrid, these rarely change and take a noticeable amount of time to compute on larger designs (from this weekends testcase, about 45 seconds was spent repeatedly calculating this value in PDN (of course that is an inefficiency there, but this solves it for all)). When the grid is modified the cache is cleared.
  • make gui/pad/pdn use returned references to the grid vector.

Signed-off-by: Peter Gadfort <peter.gadfort@gmail.com>
Signed-off-by: Peter Gadfort <peter.gadfort@gmail.com>
@gadfort gadfort requested a review from maliberty November 17, 2025 14:23
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

Signed-off-by: Peter Gadfort <peter.gadfort@gmail.com>
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@maliberty
Copy link
Member

Its not that much memory but you'll still pay the inefficiency once. If it is faster there may be no need to cache. There is no reserve call, no need to sort for a single pattern, even multi-pattern could be more efficient than an append and sort.

@gadfort
Copy link
Collaborator Author

gadfort commented Nov 17, 2025

@maliberty I'm not sure what you mean by faster? The memory is trivial, and the improvements you are suggesting can be implemented, in a different PR.

@maliberty
Copy link
Member

By faster I mean efficient enough that no cache is needed. Caching tends to be a source of bugs so I avoid it if I can (for instance this PR missed create / destroy invalidation).

@gadfort
Copy link
Collaborator Author

gadfort commented Nov 17, 2025

On create, there is nothing to do (the grid vector would be empty, thus load loaded).
On adding pattern, the grid is cleared to ensure it would be created again if requested.
On destroy the object is destroyed along with its members.
I don't think there is any risk here of a invalid cache (if you see something that is missing specifically, then I can address that).

Allocating the memory over and over again is also not particularly efficient. In the profile, insertion still takes real time (at scale).

auto* tracks = block->findTrackGrid(layer_);
if (dir == odb::dbTechLayerDir::HORIZONTAL) {
tracks->getGridY(grid_);
grid_ = tracks->getGridY();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are making a copy here fwiw

@maliberty
Copy link
Member

I think this is unnecessary but not worth arguing over.

@maliberty maliberty merged commit 3536fae into The-OpenROAD-Project:master Nov 17, 2025
13 checks passed
@gadfort gadfort deleted the odb-cache-grids branch November 17, 2025 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants