[suhailpatel]

We are calling PrefixMatches which takes a copy of the terr when we don't need to, avoiding this saves a bunch of allocations

                  │ /tmp/bench_before.txt │        /tmp/bench_after.txt         │
                  │        sec/op         │   sec/op     vs base                │
New-14                        1.706µ ± 3%   1.437µ ± 2%  -15.74% (p=0.000 n=10)
AugmentGoError-14             1.639µ ± 1%   1.366µ ± 2%  -16.66% (p=0.000 n=10)
AugmentTerror-14              1.321µ ± 1%   1.310µ ± 3%        ~ (p=0.099 n=10)
geomean                       1.545µ        1.370µ       -11.35%

                  │ /tmp/bench_before.txt │          /tmp/bench_after.txt          │
                  │         B/op          │     B/op      vs base                  │
New-14                       2.642Ki ± 0%   1.907Ki ± 0%  -27.80% (p=0.000 n=10)
AugmentGoError-14            2.657Ki ± 0%   1.923Ki ± 0%  -27.64% (p=0.000 n=10)
AugmentTerror-14             2.251Ki ± 0%   2.251Ki ± 0%        ~ (p=1.000 n=10) ¹
geomean                      2.509Ki        2.021Ki       -19.46%
¹ all samples are equal

                  │ /tmp/bench_before.txt │         /tmp/bench_after.txt         │
                  │       allocs/op       │ allocs/op   vs base
New-14                        14.000 ± 0%   5.000 ± 0%  -64.29% (p=0.000 n=10)
AugmentGoError-14             15.000 ± 0%   6.000 ± 0%  -60.00% (p=0.
AugmentTerror-14               8.000 ± 0%   8.000 ± 0%        ~ (p=1.000 n=10) ¹
geomean                        11.89        6.214       -47.72%

[EddShaw]

I was going to say:

I'm a bit worried that if we're no longer unconditionally copying the params map, then the caller of the terrors constructor can keep a reference to this map around and modify it after the fact.

But that's not even what we were doing before, so it must be fine. 🙈

[suhailpatel]

This tripped me up too but yeah the behaviour is preserved

[EddShaw]

Ah, took me a moment to clock that the allocation is coming from the free function PrefixMatches, which takes an error. Error.PrefixMatches doesn't Wrap to access the error code, avoiding an allocation.

I think we can avoid the allocation in PrefixMatches as well - we don't need to Wrap. We could use the same type-cast as in WrapWithCode. That'll benefit PrefixMatches callers too. But it might not be worth the marginal increase in complexity?

[EddShaw]

Nice.

It'd be cool if CI could track benchmark results over time, or if we could assert on things like allocations, but definitely not for this PR. 🙈