Skip to content

Commit

Permalink
Use objectpath.Encoder to amortize the cost of encoding the paths of …
Browse files Browse the repository at this point in the history
…multiple objects (#140)

We are using `objectpath.For` very heavily in our inference engine to
construct package-independent object identifiers. However, each
invocation comes with a cost of initializations (construction and
traversal of the scope tree).

Since x/tools v0.9.0 (we are currently on v0.11.0), there is an
`objectpath.Encoder` that "[amortizes the cost of encoding the paths of
multiple
objects](https://pkg.go.dev/golang.org/x/tools/go/types/objectpath#Encoder)".
This PR uses the encoder for better performance. There aren't/shouldn't
be any functionality changes.

A quick profile showing the timing before & after this PR for
`accumulation.run` function (the entrypoint function for our inference
engine):

before:

```
31.43s 22.49% |   go.uber.org/nilaway/accumulation.run
15.69s 49.92% |   go.uber.org/nilaway/inference.(*Engine).ObservePackage
8.11s 25.80%  |   go.uber.org/nilaway/inference.(*Engine).ObserveUpstream
3.33s 10.59%  |   go.uber.org/nilaway/inference.(*InferredMap).Export
...
```

after:

```
18.34s 14.63% |   go.uber.org/nilaway/accumulation.run
8.55s 46.62%  |   go.uber.org/nilaway/inference.(*Engine).ObserveUpstream
3.65s 19.90%  |   go.uber.org/nilaway/inference.(*InferredMap).Export
2.50s 13.63%  |   go.uber.org/nilaway/inference.(*Engine).ObservePackage
...
```
  • Loading branch information
yuxincs authored Nov 30, 2023
1 parent f9441c8 commit 0ef3071
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion inference/primitive.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ type primitivizer struct {
// curDir is the current working directory, which is used to trim the prefix (e.g., random
// sandbox prefix if using bazel) from the file names for cross-package references.
curDir string
// objPathEncoder is used to encode object paths, which amortizes the cost of encoding the
// paths of multiple objects.
objPathEncoder *objectpath.Encoder
}

// newPrimitivizer returns a new and properly-initialized primitivizer.
Expand Down Expand Up @@ -172,6 +175,7 @@ func newPrimitivizer(pass *analysis.Pass) *primitivizer {
pass: pass,
upstreamObjPositions: upstreamObjPositions,
curDir: cwd,
objPathEncoder: &objectpath.Encoder{},
}
}

Expand All @@ -193,7 +197,7 @@ func (p *primitivizer) fullTrigger(trigger annotation.FullTrigger) primitiveFull

// site returns the primitive version of the annotation site.
func (p *primitivizer) site(key annotation.Key, isDeep bool) primitiveSite {
objPath, err := objectpath.For(key.Object())
objPath, err := p.objPathEncoder.For(key.Object())
if err != nil {
// An error will occur when trying to get object path for unexported objects, in which case
// we simply assign an empty object path.
Expand Down

0 comments on commit 0ef3071

Please sign in to comment.