Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Diffing #12

Merged
merged 10 commits into from Dec 7, 2015
Merged

Diffing #12

merged 10 commits into from Dec 7, 2015

Conversation

@vasilisp
Copy link
Contributor

@vasilisp vasilisp commented Oct 18, 2015

This patch implements a diffing algorithm. When a "set" operation is performed (either directly by the client, or via the implementation of make_from_s) we diff the old data structure contents with the new contents, and export incremental patches.

The diffing algorithm for lists is based on computing the longest common subsequence (based on standard recursion plus memoization). This is of quadratic complexity in the worst case, but I suspect our implementation performs decently in practice.

Most functions have gained an ?eq argument, which may be crucial for diffing; it depends on the contents. Note that diffing happens "early", e.g., if the input is a list of integers which is turned into a list of DOM nodes via map, then we diff lists of integers. In this example, there is no need to pass ~eq (the default (=) is just fine for integers). This early diffing means that we can avoid RPCs, DB queries, rebuilding DOM nodes and other complicated operations that happen downstream.

As a side-effect, the mutable lists kept internally have been replaced by signals. I think the new implementation of fold fixes #9.

@Drup
Copy link
Member

@Drup Drup commented Oct 18, 2015

This is great!
Since we have a diffing operation, we should change the API: make make_from_s the main entry function (and rename everything in a consistent fashion).

Also, I think we need a bench suite.

@Drup
Copy link
Member

@Drup Drup commented Oct 18, 2015

Also, please rename eq to equal, to make all the deriving-like libraries happy.

@vasilisp
Copy link
Contributor Author

@vasilisp vasilisp commented Oct 19, 2015

@Drup, React uses eq everywhere, and ReactiveData is closely tied to React. The benefit in connection to deriving would be marginal anyway, since we do not produce equality functions.

@@ -24,6 +24,8 @@ module type DATA = sig
val map_patch : ('a -> 'b) -> 'a patch -> 'b patch
val map_data : ('a -> 'b) -> 'a data -> 'b data
val empty : 'a data
val eq : ('a -> 'a -> bool) -> 'a data -> 'a data -> bool
Copy link
Member

@Drup Drup Oct 19, 2015

I meant this equal, not the labels.

@vasilisp
Copy link
Contributor Author

@vasilisp vasilisp commented Dec 7, 2015

Any further comments on this? It needs to get merged before the long-awaited Eliom release.

@Drup
Copy link
Member

@Drup Drup commented Dec 7, 2015

Let's merge, I'll try to come up with a good API later.

@vasilisp vasilisp merged commit c456dcb into ocsigen:master Dec 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants