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

Node insertion optimization #243

Conversation

ivan-tymoshenko
Copy link
Collaborator

@ivan-tymoshenko ivan-tymoshenko commented Mar 28, 2022

As I mention here #242, it wasn't the best solution to this #241 issue. This is much better. I needed to optimize the insert method first. The main idea of optimization is that previously the insertion of each node started at the root of the tree. Now, setting a new node is based on the previous one. This allowed us to simplify the code a lot.

@ivan-tymoshenko ivan-tymoshenko force-pushed the refactoring-insert-method branch from 5ef0f10 to 28dc50d Compare March 28, 2022 20:58
Copy link
Collaborator

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina requested a review from delvedor March 28, 2022 21:13
Copy link
Contributor

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

Can you share the benchmarks?

Copy link
Owner

@delvedor delvedor left a comment

Choose a reason for hiding this comment

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

Nice! Can you share the benchmarks?

@ivan-tymoshenko
Copy link
Collaborator Author

Main branch

Снимок экрана 2022-03-29 в 12 13 51

Current branch

current

Copy link
Owner

@delvedor delvedor left a comment

Choose a reason for hiding this comment

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

LGTM

@mcollina
Copy link
Collaborator

mcollina commented Apr 6, 2022

Could you rebase? Thanks!

@ivan-tymoshenko
Copy link
Collaborator Author

@mcollina #237 depended on this. Since you've merged it we, don't need to merge this anymore.

image

@mcollina mcollina closed this Apr 6, 2022
@ivan-tymoshenko ivan-tymoshenko deleted the refactoring-insert-method branch April 6, 2022 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants