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

Optimize trie operations by replacing tail recursion with iteration #149

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

visitorckw
Copy link
Contributor

Replace tail-recursive calls in insert_trie() and find_trie() with an iterative loop to reduce function call overhead and avoid potential stack overflow issues. The logic remains functionally equivalent, improving performance and stability.

Replace tail-recursive calls in insert_trie() and find_trie() with an
iterative loop to reduce function call overhead and avoid potential
stack overflow issues. The logic remains functionally equivalent,
improving performance and stability.
@jserv jserv requested a review from vacantron August 11, 2024 16:46
* way to handle this is to dynamically allocate a new element.
*/
trie->next[fc] = func_tries_idx++;
for (int i = 0; i < 128; i++)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure if 128 is a proper range for such purpose. Can you determine?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact, the names of C functions can only contain a total of 63 characters: underscores, digits 0-9, and letters a-z and A-Z. Therefore, using 128 next entries for each node is indeed a waste of memory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

However, I think we might be able to address this issue in a later patch since it differs from the objective of this patch?

Copy link
Collaborator

Choose a reason for hiding this comment

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

However, I think we might be able to address this issue in a later patch since it differs from the objective of this patch?

up to you. We are experiencing performance issues during bootstrapping, and it would be beneficial to identify and resolve these to speed up the compilation process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, I think using a radix tree here could further improve memory savings. I can give it a try later.

@jserv jserv merged commit 1586eb1 into sysprog21:master Aug 14, 2024
4 checks passed
@jserv
Copy link
Collaborator

jserv commented Aug 14, 2024

Thank @visitorckw for contributing! Let's move forward.

@visitorckw visitorckw deleted the trie-tail-call-optimization branch August 14, 2024 19:29
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.

2 participants