Various improvements to performance and stability

[howardjohn]

This PR introduces a series of patches with the ultimate goal of improving performance. In our use case, we see this library as bottleneck - applying single patches can take >30s and allocate GBs of data against relatively small objects (Kubernetes objects size, not like GBs of json or anything crazy).

In the process of fixing this, I improved the test coverage by adding fuzz tests and benchmarks to ensure the changes I made actually improved performance, and did not cause regressions.

As part of this, the fuzz tests found a few bugs around edge case handling which are also fixed:

• Empty keys ("") handling
• JSON marshaling of keys that need to be escaped

Finally, the compareEditDistance optimization is removed. This is what brings a (massive) performance improvement, while still passing all round-tripping tests (including the new fuzz tests, which gives a high degree of confidence). The function as written introduces n^2 behavior, behaving poorly on large array inputs. Simply removing this appears to pass all tests, so I took that approach.

Results:

name                        old time/op    new time/op    delta
CreatePatch/complex-48         167µs ± 8%     156µs ± 4%   -6.85%  (p=0.001 n=10+10)
CreatePatch/large_array-48     664ms ± 1%       2ms ± 3%  -99.71%  (p=0.000 n=10+10)
CreatePatch/simple-48         2.95µs ± 2%    2.92µs ± 1%     ~     (p=0.447 n=10+10)

name                        old alloc/op   new alloc/op   delta
CreatePatch/complex-48        75.8kB ± 0%    75.0kB ± 0%   -0.95%  (p=0.000 n=10+10)
CreatePatch/large_array-48     153MB ± 0%       1MB ± 0%  -99.39%  (p=0.000 n=9+10)
CreatePatch/simple-48         1.23kB ± 0%    1.23kB ± 0%   +0.04%  (p=0.033 n=10+10)

name                        old allocs/op  new allocs/op  delta
CreatePatch/complex-48         1.20k ± 0%     1.17k ± 0%   -2.41%  (p=0.000 n=10+10)
CreatePatch/large_array-48     7.01M ± 0%     0.01M ± 0%  -99.79%  (p=0.000 n=10+10)
CreatePatch/simple-48           29.0 ± 0%      29.0 ± 0%     ~     (all equal)

This patch is applied to the v2 branch only, rather than v3, as the usage of ordermap in the v3 branch is a blocker for us due to similar performance concerns. If desired, I can port the test/bugfix improvements to v3 as well, although we would not personally use it.\

Fuzzing results:

fuzz: elapsed: 1h40m51s, execs: 1706762529 (242296/sec), new interesting: 205 (total: 942

[CLAassistant]

All committers have signed the CLA.

[howardjohn]

Please LMK what you think @tamalsaha . Thanks!

[tamalsaha]

Why is this part not needed?

[howardjohn]

The test case for this is:

	emptyKeyA = `{"":[0]}`
	emptyKeyB = `{"":[]}`

The patch should be

[
    {
        "op": "remove",
        "path": "//0"
    }
]

Before this change, we would have the path /0. which was not correct.

I am not sure why it was originally added. Removal of this code did not impact any existing test cases, and the fuzz tester should verify we round-trip successfully. So I think the only case this code was hit was for the empty key edge case

[tamalsaha]

One question, otherwise LGTM.

[tamalsaha]

Tagged https://github.com/gomodules/jsonpatch/releases/tag/v2.3.0

Thanks a lot for your contribution!