Skip to content
This repository has been archived by the owner on May 25, 2021. It is now read-only.

Significant performance improvement due to removal of deep clone operations inside of JSON diff/patch #573

Merged
merged 4 commits into from
Aug 29, 2016

Conversation

clbond
Copy link
Contributor

@clbond clbond commented Aug 29, 2016

In this PR I have created a copy of a library that we depended upon, https://github.com/Starcounter-Jack/JSON-Patch, made a fork of it, made significant performance improvements to it, and then replaced our dependency on that library with our local fork.

The biggest performance improvement comes from the removal of the deep cloning operations that were happening inside the JSON patch library. These were completely unnecessary in the context of our use-case and introduced a significant performance bottleneck.

@mention-bot
Copy link

@clbond, thanks for your PR! By analyzing the annotation information on this pull request, we identified @sumitarora, @ericjim and @vanessayuenn to be potential reviewers

@@ -0,0 +1,284 @@
// This code contains work that is copyrighted to Joachim Wester (2013)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clbond I am assuming that only the minimal code that is needed for Augury was forked into this file from the original repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, the bare minimum

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@igor-ka
Copy link

igor-ka commented Aug 29, 2016

@clbond LGTM :shipit:

PS. consider submitting a PR to the original repo if it makes sense

@clbond clbond merged commit 6eedb45 into rangle:dev Aug 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants