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

Add entriesTail method to RBTree #325

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jzxchiang1
Copy link
Contributor

Inspired by Java's TreeMap#tailMap method but this one automatically gives you an iterator.

The use case is cursor pagination through a long list.

I've been trying to implement cursor pagination in the IC. Think a long list of items that a client needs to fetch, e.g. comments on a post, stories on a feed, etc. The client should fetch a small number of items at a time, say 20, in order to minimize server load as well as meet IC message requirements (<2 MB for requests, I think).

Offset pagination is one option, but it's not really viable at all for real-time, dynamic data. If items get removed or added while the user is scrolling through a partial list, it can result in skipped or duplicate data rendered on the client. At least there isn't really a performance hit on the IC, since it can be implemented with array indexing thanks to orthogonal persistence.

As a result, cursor pagination is generally preferred. In web2 land, this is generally done with a SQL query WHERE id > ... LIMIT .... This requires id be sorted in some database index. On the IC, the only data structure that can emulate a sorted map like this is RBTree.

So in this PR, I add a new method to the RBTree.RBTree class that allows a user to pass in some cursor (i.e. x) to continue iterating where they left off, crucially in ~O(log n) time.

@jzxchiang1
Copy link
Contributor Author

@matthewhammer might be a good candidate to review, given that he wrote the RBTree.mo code. If someone could approve the workflow run (for tests), that would also be great. Thanks!

@jzxchiang1
Copy link
Contributor Author

Can you please run it again?

@ggreif
Copy link
Contributor

ggreif commented Jan 4, 2022

My (minor) concern is that having such a method will add the need to implement similar methods for other container also. Not necessarily a bad thing, but a library designer should make a statement about this.

case (#greater) {
trees := ?(#tr(r), ts);
next()
};
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: semicolons before closing braces are redundant.

@ggreif ggreif changed the title Add entriesTail method to RBTree Add entriesTail method to RBTree Jan 4, 2022
@ggreif ggreif changed the title Add entriesTail method to RBTree Add entriesTail method to RBTree Jan 4, 2022
@jzxchiang1
Copy link
Contributor Author

My (minor) concern is that having such a method will add the need to implement similar methods for other container also. Not necessarily a bad thing, but a library designer should make a statement about this.

Fair enough, but I think this method is somewhat unique to RBTree, since it's the only ordered container available in motoko-base right now.

@dfinity-droid-prod
Copy link

Dear @jzxchiang1,

In order to potentially merge your code in this open-source repository and therefore proceed with your contribution, we need to have your approval on DFINITY's CLA1.

If you decide to agree with it, please visit this issue and read the instructions there.

— The DFINITY Foundation

Footnotes

  1. Contributor License Agreement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants