Skip to content

Commit

Permalink
Refactor some visitor internals (p4lang#4447)
Browse files Browse the repository at this point in the history
 - Refactor `Inspector`'s visiting tracking to be similar to `ChangeTracker` of `Modifier` / `Transform`. Albeit very similar, there are important differences here and there
 - Fixed subtle iterator invalidation bug in `Inspector::apply_visitor`. It was not exposed due to way `std::unordered_map` is implemented in libc++ / libstdc++. Long story short: iterator was kept across possible insertion. It will be invalidated in case of rehash
 - Ensured no internal guts of visiting process is exposed for the purposes of `visitOnce` / `visitAgain`. Previously we had a pointer to a field inside map's value stored in `Visitor`. So we risked a lot to obtain some dangling pointer to freed memory. It worked due to guarantees we had from `std::unordered_map`, but would stop to work with maps with less strong guarantees. And in general, it looks like an escaping pointer complicating code clarity.
 - Reduced size of `Visitor::Context` via rearranging fields to reduce alignment padding waste
 - Removed extraneous map lookups from hot code paths

As a result we are having ~5% speedup on `P4CParserUnroll.switch_20160512` testcase
  • Loading branch information
asl authored Feb 23, 2024
1 parent 2312c47 commit 05e2e5d
Show file tree
Hide file tree
Showing 2 changed files with 261 additions and 124 deletions.
Loading

0 comments on commit 05e2e5d

Please sign in to comment.