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

Sub-Module the functional-red-black-tree library #203

Closed
MeirionHughes opened this issue Jul 29, 2020 · 3 comments
Closed

Sub-Module the functional-red-black-tree library #203

MeirionHughes opened this issue Jul 29, 2020 · 3 comments
Labels
wontfix This will not be worked on

Comments

@MeirionHughes
Copy link
Member

MeirionHughes commented Jul 29, 2020

I propose adding functional-red-black-tree either as a sub-module of memdown or simply duplicate the code into here. The library hasn't been meaningfully updated in 6 years.

Specifically it would open up the door to making changes to improve performance: specifically replacing the current find + update or insert with an upsert method; i.e. cut tree-traversal by half for new inserts.

I think there would also be an opportunity, in the future, to improve batch loading too.

alternatively, we could switch to functional-red-black-tree2 as @Kirill486 seems to have picked up the torch.

@MeirionHughes MeirionHughes added the discussion Discussion label Jul 29, 2020
@MeirionHughes MeirionHughes changed the title Vendor the functional-red-black-tree library Sub-Module the functional-red-black-tree library Jul 30, 2020
@vweevers
Copy link
Member

The library hasn't been meaningfully updated in 6 years.

A testament to its quality 😉 I see no reason to believe that the project is dead (the open PRs are only related to TypeScript). If it is, I would prefer that someone takes over maintenance. In any case, I'm opposed to vendoring. Any improvements will also benefit the module by itself - which I've used in the past as a synchronous alternative to memdown.

Let's first just try PRs to functional-red-black-tree, for example for an upsert method.

@MeirionHughes
Copy link
Member Author

okay fair enough. I spent some time trying to get into it last night; background reading on "purely functional" BST and trying to see how its done in the lib. need to do some more playing to fully understand the path-stacks though.

@MeirionHughes MeirionHughes added wontfix This will not be worked on and removed discussion Discussion labels Jul 30, 2020
@MeirionHughes
Copy link
Member Author

@vweevers yeah I see what you mean. its rock solid. Its performance is hard to beat - by a wide margin. thought I'd give a functional avl a go: I can't get close. even for basic find I'm 50% slower. :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants