Skip to content

Remove recursion from Delaunator::legalize #9

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

Closed
delfrrr opened this issue Sep 18, 2018 · 3 comments
Closed

Remove recursion from Delaunator::legalize #9

delfrrr opened this issue Sep 18, 2018 · 3 comments

Comments

@delfrrr
Copy link
Owner

delfrrr commented Sep 18, 2018

data controlled recursions can case stack overflows
cc @cleeus

@flippmoke
Copy link
Contributor

Recursion is often a valid design philosophy, I am not quite sure why you removed it @delfrrr -- is there some particular case it was often failing?

@delfrrr
Copy link
Owner Author

delfrrr commented Oct 30, 2018

well, as I understand uncontrolled recursion can case stack overflows, as it stated earlier, plus it will create and keep more variables comparing to current implantation;

@flippmoke
Copy link
Contributor

I reverted back to recursion in my restructured design test (that started before this landed) in #14. I am going to think about how to remove it, but the extra initialization of a vector seems to be rather heavy and might cause even more allocations. There is no reason as well that this vector couldn't grow in endlessly in size (just like your stack) in the event of an endless loop.

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

No branches or pull requests

2 participants