Use Kahn's algorithm for CRDT toposort #49
Conversation
The existing implementation is O(n^2) in the number of nodes. That's fine for drawings, but it takes minutes to process big text documents because the reMarkable creates a node for each word. This patch implements [Kahn's algorithm](https://en.wikipedia.org/wiki/Topological_sorting#Kahn's_algorithm). That would ordinarily be O(n), but to maintain deterministic ordering for concurrent edits (as the current algorithm does), we use a heap instead of a set, making the runtime O(n log n). My text-heavy notebook now processes in about a second instead of minutes.
|
This is amazing! There are two primary bottlenecks I've encountered in processing rM documents to PDF. Toposort is the first, PDF generation the second. This makes the former negligible, leaving only PDF generation as a slow part of the pipeline! |
|
I'm not sure if @ricklupton is still actively maintaining rmsene and rmc. I've been maintaining forks at https://github.com/scrybbling-together/rmscene.git, I aim to review the code myself when I have time soon, hopefully this weekend. |
|
I'm not deep enough in this library to know, but it's possible we could get away with using sets rather than heaps—I don't know if it's actually important to maintain deterministic ordering for CRDT nodes which aren't properly ordered by their edges. I just maintained that to match the existing behavior. Probably the extra log(N) factor isn't all that meaningful—haven't tested. |
|
I have a dataset of real-world .rm files, I sampled a bunch of them to get data. The O(n^2) labels are the original algorithm, and the O(n log n) are yours. 
The impact is mostly at the tail, so for larger files, processing is much faster. I also ran a simulation sampling 10000 files randomly, which is what is bottom two graphs. I do think optimizing further to Note, this was a dataset of real files and there were 0 errors, so that's a good sign. In my opinion this is ok to merge. |
2 participants
The existing implementation is O(n^2) in the number of nodes. That's fine for drawings, but it takes minutes to process big text documents because the reMarkable creates a node for each word.
This patch implements Kahn's algorithm. That would ordinarily be O(n), but to maintain deterministic ordering for concurrent edits (as the current algorithm does), we use a heap instead of a set, making the runtime O(n log n).
My text-heavy notebook now processes in about a second instead of minutes. I've verified that the tests pass as before.