search

idubrov / json-patch

RFC 6902 (JSON Patch) / RFC 7396 (JSON Merge Patch) implementation for Rust
Apache License 2.0
127 stars 17 forks source link

refactor: do not use recursion but keep a stack #28

Closed baszalmstra closed 8 months ago

baszalmstra commented 8 months ago

When using this library we consistently ran into stack overflow issues. We traced this back to this crate which uses recursion to be able to revert operations in case of an error. This is problematic when applying large patches because it will quickly overflow the stack.

This PR does two things:

  1. Introduces a stack of undo operations which is constructed as the patches are applied. The stack contains the inverse operations of the patches. If an error occurs this stack is unwound to reconstruct the original document. This effectively removes recursion and stores the undo stack on the heap instead of on the stack.
  2. Added (back) a patch_unsafe function which does not construct this undo stack and in turn leaves the document in an inconsistent state.

In our case, we are applying large patches and we don't care at all if the document is in an inconsistent state in case of an error. We throw it away anyway. With this PR this significantly reduces memory usage and increases the performance of applying the patches.

baszalmstra commented 8 months ago

Thanks! I've applied your suggestion.

I'm not sure why the test was failing. The only change in the schema was that a description was added for PatchOperation which already existed in the code but was previously missing in the schema. I've updated the test reference to reflect this.

baszalmstra commented 8 months ago

Now it seems like codecov is failing.. I think the github action needs to be updated perhaps?

idubrov commented 8 months ago

Sorry about that. Yes, it turned out the test was broken on main and codecov configuration stopped working (it didn't have the credentials). I fixed both of these issues on main, so if you rebase, this PR should become green.