The `json.patch` built-in is quite versatile, and compared to
patching via e.g. `object.union` et. al. often communicates
intent better, IMO. But while it uses some fairly advanced
logic for complex patch operations, it doesn't perform all that
great on simple ones. This is a first and pretty basic attempt
to improve that somewhat by picking the most low-hangig performance
fruits, like avoiding repeated allocations of temporary term pointers.
The main allocation source is the creation of EditTree's, and this
remains a problem. I have created a sync pool but only managed to
get the outermost edit tree to recycle, as I found it really hard
to track where it's safe to release those created in the deeply
nested calls. Additionally, I managed to trigger stack overflows
trying to recycle child trees, so there seems to be some circular
refs? Or I just did something wrong.
If someone wants to look into this and pick up where
I left, that'd be great!
- Add InternedIntRange for testing, primarily
- Intern keys used in json.patch patches
- Clean up json.X built-in benchmarks
- Reduce allocations in edit tree function
- Avoid using intermediate data structures
for JSON patches
- Some unrelated interning fixes to reduce noise
in tests and benchmarks (e.g. do less stuff in
var inits)
Selected benchmark that I used while working on this:
**Before**
```
BenchmarkJSONPatchAddShallowScalar/object-10-16 147853 8008 ns/op 9667 B/op 206 allocs/op
BenchmarkJSONPatchAddShallowScalar/array-10-16 201704 5889 ns/op 7256 B/op 173 allocs/op
BenchmarkJSONPatchAddShallowScalar/set-10-16 182566 6733 ns/op 8103 B/op 156 allocs/op
```
**After**
```
BenchmarkJSONPatchAddShallowScalar/object-10-16 197414 6066 ns/op 7256 B/op 133 allocs/op
BenchmarkJSONPatchAddShallowScalar/array-10-16 278121 4427 ns/op 5285 B/op 100 allocs/op
BenchmarkJSONPatchAddShallowScalar/set-10-16 233884 4839 ns/op 6243 B/op 113 allocs/op
```
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
Originally meant to be `array.concat_n`, but this name is better
as the behavior of this function differs from `array.concat` —
namely that `array.flatten` accepts any type of valued in the
input array. Only arrays are however flattened, and the rest
are appended directly to the flattened output.
Note that this function only flattens at the topmost level of
the input array — not recursively! A cursory look
at a few other languages suggest a single level is the common case.
But if others feel we should flstten more, I'm happy to make an update.
The C code for a Wasm implementstion here is cowboy coded, and
I did not manage to run the tests on my machine due to some
`docker` <-> `container` differences. I mostly just imitated
the existing code in the array category. I doubt it'll work
on the first try, but only CI can judge me.
Also:
- Remove `opa fmt` step from the Rego CI step, as this is done by
Regal anyway a little later in the list of tasks.
- Replace some hard-coded `docker` names in the `Makefile` with `$(DOCKER)`
- Added name of built-in function missing to the unsupportedBuiltinErr
error, as it has happened a few times now that I've used `:=` in a
query, and had no clue what built-in it referred to.
Fixes#8226
Signed-off-by: Anders Eknert <anders.eknert@apple.com>
Avoid unnecessary allocations in builtinArrayReverse for trivial cases:
- Empty arrays return ast.InternedEmptyArray directly
- Single-element arrays return the original operand unchanged
This follows the same pattern used in builtinArrayConcat and
builtinArraySlice for handling edge cases.
Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
Having worked on performance improvements in OPA on the side for almost
a month now, there's a lot of code piling up 😅 So much that a single PR
would be way too much to review. Instead, I'm splitting the work into
chunks, and will submit the next PR as soon as this one is merged. Using
the same benchmark as before — Regal linting itself, these new changes
in total reduce the number of allocations by ~13 million, and quite a
substantial amount of evaluation time saved as well.
This first PR is isolated to improvements to terms, values and
built-ins, and saves ~3M allocations. The details can be found below for
each change, and of course in the code :)
**BenchmarkRegalLintingItself-10 Before**
```
1885978209 ns/op 3497157312 B/op 69064779 allocs/op
```
**BenchmarkRegalLintingItself-10 After**
```
1796255084 ns/op 3452379408 B/op 66126623 allocs/op
```
**Terms**
- Use pointer receivers consistently for object and set types. This allows
changing the sortGuard once lock from a pointer to a non-pointer type, which
is really the biggest win performance-wise in this PR.
- Comparisons happen all the time, so make sure these take the shortest path
possible whenever, possible, such as when one type is compared to another
value of the same type.
Built-in functions:
**Arrays**
- Both `array.concat` and `array.slice` will now return the operand on operations
where the result isn't different from the input operand (like when concatenating
an empty array) instead of allocating a new term/value.
**Strings**
- Return operand on unchanged result rather than allocating new term/value.
- Where applicable, have functions take a cheaper path when string is ASCII
and we can avoid the cost of rune conversion.
**Crypto**
- Hashing functions now optimized, spending less than half the time compared to
previously.
**Objects**
- Avoid heap allocating result boolean escaping its scope, and instead use the
return value of the `Until` function.
**HTTP**
- Use interned terms for keys in configuration object, avoding allocating these
each time `http.send` is invoked.
**Globs**
- Use read/write lock to avoid contention. Use package level vars for "constant"
values, avoiding them to escape to the heap each invocation.
**Not directly/only related to built-in functions**
- Add `ValueName` function replacing the previous `TypeName` functions for
getting the name of Value's without paying for `any` interface allocations.
- Add a few more interned terms.
Signed-off-by: Anders Eknert <anders@styra.com>
Anders Eknert