Cache objectpath.Encoder.For() results to reduce CPU overhead#384
Cache objectpath.Encoder.For() results to reduce CPU overhead#384ewhauser wants to merge 2 commits intouber-go:mainfrom
Conversation
Add objPathCache map[types.Object]objectpath.Path to cache the results of objectpath.Encoder.For() calls. The same types.Object can be queried multiple times during analysis (shallow/deep checks, multiple triggers referencing the same site), and each For() call performs expensive recursive type traversal via objectpath.find(). Profiling shows objectpath.find() consuming 38-70% of CPU time in nogo builds. This cache eliminates redundant traversals for repeated queries of the same object.
|
|
Building on the caching from the previous commit, this adds fast paths to skip expensive objectpath.Encoder.For() calls: 1. Unexported non-types never have valid object paths - skip immediately 2. Package-level objects have simple paths (just the name) - compute directly This increases the performance an additional ~30%
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #384 +/- ##
==========================================
+ Coverage 87.01% 87.04% +0.02%
==========================================
Files 73 73
Lines 8305 8324 +19
==========================================
+ Hits 7227 7246 +19
Misses 885 885
Partials 193 193 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // Fast path: unexported non-types never have a valid object path | ||
| _, isTypeName := obj.(*types.TypeName) | ||
| if !obj.Exported() && !isTypeName { | ||
| p.objPathCache[obj] = "" | ||
| return "" | ||
| } | ||
|
|
||
| // Fast path: package-level objects have a simple path (just the name) | ||
| if pkg := obj.Pkg(); pkg != nil { | ||
| if pkg.Scope().Lookup(obj.Name()) == obj { | ||
| path = objectpath.Path(obj.Name()) | ||
| p.objPathCache[obj] = path | ||
| return path | ||
| } | ||
| } |
There was a problem hiding this comment.
I'll admit that I don't remember the actual implementation for object path encoder, but these feel like these fast paths should just exist inside the encoder?
| if err != nil { | ||
| path = "" | ||
| } | ||
| p.objPathCache[obj] = path |
There was a problem hiding this comment.
I'm not sure if a cache should just live inside the encoder too such that all callers get such performance boost (unless upstream does not really want to add the extra memory footprint?)
If so I'm not opposed to maintaining a cache here :) (I'll check if this increases a lot of memory consumption in our internal Go repo too)
|
This is nice! I left two comments more as discussion points rather than requests, let me know what you think! |
3 participants
Add objPathCache map[types.Object]objectpath.Path to cache the results of objectpath.Encoder.For() calls. The same types.Object can be queried multiple times during analysis (shallow/deep checks, multiple triggers referencing the same site), and each For() call performs expensive recursive type traversal via objectpath.find().
Profiling shows objectpath.find() consuming 40-70% of CPU time in our internal nogo builds. This cache eliminates redundant traversals for repeated queries of the same object and decreased the latency by roughly 7-8x for some packages.