Only reset the visitedNodes if they are not empty#1646
Only reset the visitedNodes if they are not empty#1646
Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts AnnotatedTypeScanner.reset() to avoid reinitializing the visitedNodes IdentityHashMap when it is already empty, reducing unnecessary allocations during scans that don’t record any visited nodes.
Changes:
- Only reassigns
visitedNodesto a newIdentityHashMapwhen the current map is non-empty.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Instead of re-using the same visitedNodes instance and clear-ing it, profiling showed it | ||
| // to be more efficient to create a new instance. | ||
| // visitedNodes.clear(); | ||
| visitedNodes = new IdentityHashMap<>(); | ||
| if (!visitedNodes.isEmpty()) { | ||
| visitedNodes = new IdentityHashMap<>(); | ||
| } |
There was a problem hiding this comment.
The comment above reset() says profiling showed it is more efficient to create a new visitedNodes instance, but the new logic only does so when the map is non-empty. Please update the comment to reflect the conditional behavior (or adjust the code to match the comment), otherwise it becomes misleading for future maintainers.
|
@copilot Can you measure whether this has any performance benefit compared to CI running on master? |
Measured this. For Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
No description provided.