Mostly automated fixes from running:
```
go run golang.org/x/tools/go/analysis/passes/modernize/cmd/modernize@latest --fix ./...
```
But carefully reviewed, and several fixes reverted as they looked like
they potentially could be less performant, and in a few cases due to
bugs in the analyzer that changed semantics of the code. Will report
these upstream.
Mostly good fixes though!
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
* server: port compile API
Also adds e2e tests: These include coverage for ucast in the prisma
setting, and thus require some JS runtime.
* e2e: selectively skip e2e Compile API tests
...for macos runs, and for the go-compat suites.
* server: accept timer_rego_external_resolve_ns metrics with value 0
When running the tests in a loop for a while, I would see values of 0ns
for this metric. However, comparing with its non-zero values, which are
often 41 or 42ns, it seems like this is just not happening in this code
path. So if "almost nothing" actually goes below 1ns, it's OK.
* e2e: split dep-heavy e2e tests into their own go module
* Makefile: export DOCKER_RUNNING (make e2e read it)
---------
Co-authored-by: Philip Conrad <philip@chariot-chaser.net>
Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Since we don't use the HTTP API in Regal, I hadn't looked at this from
a performance POV before, and it was a fun side quest :)
A pretty decent reduction of the baseline cost for the most common
request type, v1/data POST. Changes:
- Add NoOp implementation of `Metrics` for when metrics aren't needed
- Cheaper `metrics.New` instantiation avoiding unnecessary lock
- Avoid cost of decision logging if decision logging isn't enabled
- Only call request.URL.Query() if URL.RawQuery isn't empty, avoiding
allocating an empty map with each request
While the target was v1/data POST handling, some of the changes above
have a positive impact on all or most handlers. All changes have been
run through the existing tests, and a new benchmark to measure the impact
of the fixes have been added, showing:
```
13063 ns/op 15162 B/op 195 allocs/op - main
12796 ns/op 14856 B/op 189 allocs/op - avoid r.URL.Query() when no query provided
12541 ns/op 14483 B/op 187 allocs/op - decisionLogger.Log early exit if not enabled
12133 ns/op 14235 B/op 180 allocs/op - get revisions and init logger only if needed
11098 ns/op 14207 B/op 180 allocs/op - more efficient metrics.New() (without locking)
10683 ns/op 13171 B/op 169 allocs/op - use no-op metrics implementation when metrics aren't requested
```
Even if the impact is pretty good, it's worth noting that most of the improvements
above are only seen when decision logging is turned off. While this is the common case
for development, it's not in production. Getting the baseline cost down is important still
as there should be no cost paid for features unused.
Signed-off-by: Anders Eknert <anders@styra.com>
This commit fixes a potential bug that could occur with
`metrics.Timer`. Whenever `timer.Stop()` was called, the
timer would add the current delta from the start time to its
accumulated value so far, *regardless of whether it had been
restarted with `timer.Start()` or not*.
This could lead to confusing metrics in some places if `timer.Stop()`
was accidentally called twice. (Value would be much larger than
expected.)
The fix is to use a guard around the accumulating step.
Signed-off-by: Philip Conrad <philip@chariot-chaser.net>
Co-authored-by: Anders Eknert <anders@styra.com>
And many smaller performance improvements. The indexer recycling results
is one of the most impactful performance improvements as of yet, and alone
saves more than 2 million allocations in the Regal lint benchmark. The indexer
is also more efficient, as `values` are no longer stored on the struct. Thanks
@tsandall for that code!
Also included a bunch of small improvements from my perf branches.
**Before**
```
1209043041 ns/op 3255157224 B/op 64026192 allocs/op
```
**After**
```
1197131792 ns/op 3194124864 B/op 61876276 allocs/op
```
Signed-off-by: Anders Eknert <anders@styra.com>
Anders Eknert