Improve Performance of the Type System #824

diegommm · 2025-08-29T13:12:54Z

Improvements to the Type System:

Aggressively cache struct, methods, and functions information.
Make the type checker reusable.
Make all methods of Nature work on *Nature instead. This avoid excessive memory copying.
Moved many type-checking from checker to nature because this prevents too much memory copying.
Optimize and cache many type checks.
Remove dead code (run staticcheck and golangci-lint).

Details for the future:

Please, keep passing Nature as value whenever possible. I made the methods work on the pointer because it avoids excessive copying that I saw in the profiling.
The use of *Cache is mandatory only for complex operations. In ast and parser it's ok to not use it because there we don't need anything special.

goos: linux
goarch: amd64
pkg: github.com/expr-lang/expr/checker
cpu: 13th Gen Intel(R) Core(TM) i7-13700H
                                            │ bench-results-old.txt │        bench-results-new.txt        │
                                            │        sec/op         │   sec/op     vs base                │
Checker/name=function_calls-20                        16.927µ ±  3%   4.444µ ± 9%  -73.75% (p=0.000 n=20)
Checker/name=unary_and_binary_operations-20            9.088µ ± 10%   4.634µ ± 5%  -49.01% (p=0.000 n=20)
Checker/name=deep_struct_access-20                     23.12µ ±  7%   13.25µ ± 8%  -42.69% (p=0.000 n=20)
geomean                                                15.26µ         6.486µ       -57.51%

                                            │ bench-results-old.txt │        bench-results-new.txt        │
                                            │         B/op          │    B/op     vs base                 │
Checker/name=function_calls-20                         0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=20) ¹
Checker/name=unary_and_binary_operations-20            0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=20) ¹
Checker/name=deep_struct_access-20                     0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=20) ¹
geomean                                                           ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                                            │ bench-results-old.txt │        bench-results-new.txt        │
                                            │       allocs/op       │ allocs/op   vs base                 │
Checker/name=function_calls-20                         0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=20) ¹
Checker/name=unary_and_binary_operations-20            0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=20) ¹
Checker/name=deep_struct_access-20                     0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=20) ¹
geomean                                                           ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

antonmedv · 2025-08-31T09:24:44Z

Awesome, it's really interesting to see the results. I think this will be very beneficial for users who use Eval path and do not precompile expressions.

checker/nature/utils.go

antonmedv · 2025-08-31T10:15:25Z

checker/nature/nature.go

+	Type reflect.Type // Type of the value. If nil, then value is unknown.
+	Kind reflect.Kind // Kind of the value.
+
+	cache *Cache


As this cache is shared, let's do not set it inside the nature itself, but pass it from the top, from the checker as function arguments.

We already have this problem and we have to set a nature cache, for example, in the following line:

base := n.Node.Nature() base.SetCache(c) // AST doesn't cache nature info base = base.Deref()

Let's not set cash inside an agent. But pass in to all functions what may use it:

base := n.Node.Nature() base = base.Deref(c) // As the deref can modify the cache.

I used your approach in the first version, but it was very difficult to use.
I tried two ways:

Modify the signature of the methods to pass a *Cache as first argument, which can be nil.

Call SetCache every time I needed it to ensure it was used.

The problems I found:

There is no reason to not use the cache, so I think it would be ok to have it be a part.

The logic to account for cache not being set every time creates a lot of complexity. Remember this happens for every method. I have to look for a suitable alternative in every case. This makes it hard to follow and verify every case.

The solution I came to:

The cache is not agent, it is also functional. It is integral to Nature. AST only creates raw Nature, but then in order to have type information we use the cache.

The cache is a proxy but it also makes it easier to centralize all the logic. This makes it easier to implement.

The cache should be local to a conf.Config. This is because that's the best way to reuse it throughout the lifetime of a program being compiled. Starting with the type checker, all following compilation steps need it.

In the early stages when we are parsing the program we are just building the AST so we only care about creating the basics. For example, we don't know yet what type is returned at this point. But then we bind the AST to a cache then it means we are finally interested in type information details.

I think "bind" is a better word so I changed from SetCache to Bind to express this relationship.

The cache is not only meant for the type checker. I only changed mainly the type checker because it's the core, but it is also meant to be used by the optimizer, patchers, and the compiler. I also saw that we could probably use it in the vm and in the runtime because this would avoid some costly reflection checks.

I didn't go into more usage of it because I made many changes already. I would like to see if these changes are ok and then take a look at the other packages.

We already have this problem and we have to set a nature cache, for example, in the following line:

So as I explain above, this is not a problem. In AST we don't care about type information, we only know about raw elements. So when we need more information we need to bind it with Bind to a cache.

conf/env.go

antonmedv · 2025-08-31T10:16:14Z

conf/config.go

@@ -34,6 +34,7 @@ type Config struct {
 	Functions FunctionsTable
 	Builtins  FunctionsTable
 	Disabled  map[string]bool // disabled builtins
+	NtCache   nature.Cache


Same here, if we can pass it via the function inside the compiler, from the top it will be easier and we don't need to set it here.

See the other comment.

antonmedv · 2025-08-31T10:17:48Z

benchstat old.txt new.txt 
goos: darwin
goarch: arm64
pkg: github.com/expr-lang/expr/checker
cpu: Apple M2
                                           │   old.txt    │               new.txt               │
                                           │    sec/op    │   sec/op     vs base                │
Checker/name=function_calls-8                18.788µ ± 3%   7.443µ ± 0%  -60.38% (p=0.000 n=10)
Checker/name=unary_and_binary_operations-8   10.010µ ± 0%   7.997µ ± 2%  -20.11% (p=0.000 n=10)
Checker/name=deep_struct_access-8             30.04µ ± 0%   21.69µ ± 5%  -27.80% (p=0.000 n=10)
geomean                                       17.81µ        10.89µ       -38.86%

                                           │    old.txt     │                  new.txt                  │
                                           │      B/op      │     B/op      vs base                     │
Checker/name=function_calls-8                  0.000 ± 0%       0.000 ± 0%         ~ (p=1.000 n=10) ¹
Checker/name=unary_and_binary_operations-8     0.000 ± 0%       0.000 ± 0%         ~ (p=1.000 n=10) ¹
Checker/name=deep_struct_access-8            1.242Ki ± 0%     0.000Ki ± 0%  -100.00% (p=0.000 n=10)
geomean                                                   ²                 ?                       ² ³
¹ all samples are equal
² summaries must be >0 to compute geomean
³ ratios must be >0 to compute geomean

                                           │   old.txt    │                 new.txt                 │
                                           │  allocs/op   │ allocs/op   vs base                     │
Checker/name=function_calls-8                0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
Checker/name=unary_and_binary_operations-8   0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
Checker/name=deep_struct_access-8            159.0 ± 0%       0.0 ± 0%  -100.00% (p=0.000 n=10)
geomean                                                 ²               ?                       ² ³
¹ all samples are equal
² summaries must be >0 to compute geomean
³ ratios must be >0 to compute geomean

Nice -38.86% improvments. 👍🏻

diegommm · 2025-08-31T17:37:57Z

Nice -38.86% improvments. 👍🏻

I updated the benchmarks, now it's ~58%!

Awesome, it's really interesting to see the results. I think this will be very beneficial for users who use Eval path and do not precompile expressions.

This would also be useful in tight loops when we need to compile many expressions, for example when a service initializes.

antonmedv · 2025-09-01T06:13:56Z

checker/nature/nature.go

+// and lexer do not need to use the cache because they don't need any service
+// from the Nature type, they only describe. However, when receiving a Nature
+// from one of those packages, the cache must be set immediately.
+type Cache struct {


Can we use global cache instance under sync.RWLock? Will it make core more easy to maintain?

I thought of different alternatives but they seem very complicated. I will try passing Cache from the top as you suggested above.

antonmedv · 2025-09-01T06:14:45Z

checker/nature/nature.go

+
+	cache *Cache
+	*Optional
+	*FuncData


Please also write the comment on Why? do we need separate *Optional and *FuncData so reader will understand it.

antonmedv · 2025-09-01T06:15:55Z

checker/nature/nature.go

+
+	*structData
+
+	// map-only data
 	Fields          map[string]Nature // Fields of map type.


If we avoiding map in methodset , why this map?

antonmedv · 2025-09-01T06:16:48Z

checker/nature/nature.go

-	if n.Type != nil {
-		n.Type = deref.Type(n.Type)
+func (c *Cache) getStruct(t reflect.Type) Nature {
+	if c != nil {


How can c be nil here? Looks like redundent if.

antonmedv · 2025-09-01T06:19:51Z

checker/nature/nature.go

-		// Untyped nil is assignable to any interface, but implements only the empty interface.
-		if nt.IsAny() {
+		switch nt.Kind {
+		case reflect.Pointer, reflect.Interface:


Checking for reflect.Interfece is not sufficient. We need to nt.IsAny():

func (n Nature) IsAny() bool { return n.Kind() == reflect.Interface && n.NumMethods() == 0

antonmedv · 2025-09-01T06:28:10Z

checker/nature/nature.go

+	return nt
+}
+
+func (n *Nature) Bind(c *Cache) {


This is hard to follow logic. Too many ifs and branching. Why do we need to write cache on bind op?

Alos if Nature relies on Cache, lets make it impossible to create nature without the Cache?

diegommm added 20 commits August 28, 2025 18:44

export the checker and ensure no methods or fields are exported

359dd99

make Checker reusable

360157a

add PatchAndCheck method to Checker, and reuse it in visitors

d234ae3

avoid unnecessary map and consolidate structs

edce1b0

make all Nature methods use pointer-receiver

80afb76

added cache for type and nature information

1c36c13

optimize type and nature checks

e9b245b

more aggressively cache some partial results and add cheap checks

879544e

cache reused values

8f6e266

reduce copying in Nature by breaking down optional fields

1eb8ab5

enforce use of cache

06a425b

improve ergonomics of FromType and NatureOf

d9ba735

improve struct method fetching

aa33cf9

make cache private to Nature

ab994ae

first iteration improving struct reflect caching

720d2b6

cleaned up struct fields data

35e3c1e

improve caching of functions data

e68fbc7

simplify checker code

3d7e30e

simplify and remove dead code

163df86

add checker benchmarks

40d2c14

diegommm changed the title ~~[WIP] Improve Performance of the Type System~~ Improve Performance of the Type System Aug 31, 2025

diegommm marked this pull request as ready for review August 31, 2025 08:18

antonmedv requested changes Aug 31, 2025

View reviewed changes

address PR comments and minor improvements

247a6a6

diegommm added 3 commits August 31, 2025 20:12

rename from SetCache to Bind

32aebcd

make the benchmarks more fair

f6e2d87

fix benchmark option not being applied

eafdb3c

antonmedv reviewed Sep 1, 2025

View reviewed changes

antonmedv requested changes Sep 1, 2025

View reviewed changes

antonmedv reviewed Sep 1, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Improve Performance of the Type System #824

Improve Performance of the Type System #824

diegommm commented Aug 29, 2025 •

edited

Loading

Uh oh!

antonmedv commented Aug 31, 2025

Uh oh!

Uh oh!

antonmedv Aug 31, 2025

Uh oh!

diegommm Sep 1, 2025 •

edited

Loading

Uh oh!

Uh oh!

antonmedv Aug 31, 2025

Uh oh!

diegommm Sep 1, 2025

Uh oh!

antonmedv commented Aug 31, 2025

Uh oh!

diegommm commented Aug 31, 2025

Uh oh!

antonmedv Sep 1, 2025

Uh oh!

diegommm Sep 2, 2025

Uh oh!

antonmedv Sep 1, 2025

Uh oh!

antonmedv Sep 1, 2025

Uh oh!

antonmedv Sep 1, 2025

Uh oh!

antonmedv Sep 1, 2025

Uh oh!

antonmedv Sep 1, 2025

Uh oh!

Uh oh!

Uh oh!

Improve Performance of the Type System #824

Are you sure you want to change the base?

Improve Performance of the Type System #824

Conversation

diegommm commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

antonmedv commented Aug 31, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

diegommm Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antonmedv commented Aug 31, 2025

Uh oh!

diegommm commented Aug 31, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

diegommm commented Aug 29, 2025 •

edited

Loading

diegommm Sep 1, 2025 •

edited

Loading