compiler: Cache Scope + Dependence instances - #2669
Conversation
Sorry, something went wrong.
mloubout
left a comment
There was a problem hiding this comment.
minor comments, looks good
Sorry, something went wrong.
FabioLuporini
left a comment
There was a problem hiding this comment.
So can we have a breakdown of the improvements in some notable operators, such as TTI self-adjoint and examples/seismic/tti/tti_example.py?
Sorry, something went wrong.
Codecov ReportAll modified and coverable lines are covered by tests ✅ Additional details and impacted files @@ Coverage Diff @@
## main #2669 +/- ##
==========================================
- Coverage 91.99% 87.65% -4.34%
==========================================
Files 245 245
Lines 48906 48962 +56
Branches 4313 4312 -1
==========================================
- Hits 44991 42918 -2073
- Misses 3218 5309 +2091
- Partials 697 735 +38
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry.
|
Sorry, something went wrong.
EdCaunt
left a comment
There was a problem hiding this comment.
Just needs to clean up the commit names to match our internal guidelines
Sorry, something went wrong.
|
Some benchmarks attached 
|
Sorry, something went wrong.
Adds a base class CacheInstances which Scope and Dependence derive from to wrap construction behind LRU caches, allowing reuse of common dependences and equivalent scopes between cluster passes. Also applies a minor optimization within Scope.d_from_access_gen which cuts most of the time spent within that method by using set containment instead of linear searching, allowed by fixing the hash function for TimedAccess.
Tested on this self-adjoint TTI script with slight modifications, I'm seeing a consistent 25% speedup overall, largely due to reduced computation time from the set optimization as well as reuse of Scopes cutting time spent on the more expensive dependence generators. You can view a comparison of some of these hotspots as well as overall timing between running the TTI script on main versus running the same script on this branch.
This speedup is not as measurable on smaller operators such as the example scripts, but anecdotally looks to be pretty consistent.
Global Scope caching also allows us to remove the legacy QueueStateful; its use for fetching properties in analysis is factored out to a kwarg.