improve performance of recursion guard

[samuelcolvin]

The recursion guard has significant performance impact, both when validating and serialization.

We might want to change the serialisation recursion guard to be a simple depth check in future, but that would be a breaking change.

This change alters the implementation to use an array instead of a set when the guard has 16 or fewer values in it. That should be pretty common since you'd need a recursion depth > 16 to spill over into the set.

[codecov]

Codecov Report

Merging #1156 (4b82950) into main (8030c5a) will increase coverage by 0.02%.
Report is 9 commits behind head on main.
The diff coverage is 98.50%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1156      +/-   ##
==========================================
+ Coverage   89.94%   89.97%   +0.02%     
==========================================
  Files         106      106              
  Lines       16438    16482      +44     
  Branches       36       36              
==========================================
+ Hits        14785    14829      +44     
  Misses       1646     1646              
  Partials        7        7              

Files	Coverage Δ
src/recursion_guard.rs	100.00% <100.00%> (+4.34%)	⬆️
src/serializers/extra.rs	99.07% <100.00%> (-0.01%)	⬇️
src/validators/definitions.rs	94.80% <75.00%> (ø)

... and 5 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8030c5a...4b82950. Read the comment docs.

[codspeed-hq]

CodSpeed Performance Report

Merging #1156 will degrade performances by 21.52%

_{Comparing recursion-guard-speedups (4b82950) with main (d7cf72d)}

Summary

⚡ 4 improvements
❌ 2 regressions
✅ 137 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	main	recursion-guard-speedups	Change
❌	test_core_future_str	31.4 µs	39.3 µs	-20.11%
❌	test_core_future	29.4 µs	37.4 µs	-21.52%
⚡	test_json_any_list_str	1.3 ms	1.1 ms	+19.04%
⚡	test_json_any_list_int	1,200.2 µs	979.6 µs	+22.52%
⚡	test_to_json_list_of_lists	11.6 ms	9.9 ms	+17.97%
⚡	test_definition_out_of_tree	7.3 ms	5.9 ms	+24.34%

[samuelcolvin]

i don't think this is safe because there's a case where an earlier item has been removed from the array, but later values are still set.

E.g.

array = [123, 456, None, 789]

If you're looking up 789 it wouldn't be found in this case.

I had this before, but changed it to be safer.

[samuelcolvin]

I guess if we can be sure that new items are always added to the array (and removed) inside parent calls, we can be sure that there will never be None caps in the array.

@davidhewitt if you're confident of that, we can go ahead with this change. Worst case we just fallback on the depth guard.

[davidhewitt]

Pushed another commit which should work more like a stack, and we'll panic if the code is wrong.

[sydney-runkle]

V3 note from @samuelcolvin:

we should remember when it comes to V3 to remove the circular reference checks from the serialization recursion guard, it’s only really necessary in validation, and it’s quite slow make, not sure where we’re keeping a list of V3 things, but we should add this to that list

[davidhewitt]

If we don't like the MaybeUninit here we could probably use Option with only a small perf cost.

[samuelcolvin]

MaybeUninit looks graet.