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

Refactor to eliminate recursion #45

Open
pavel-kirienko opened this issue Aug 9, 2023 · 2 comments
Open

Refactor to eliminate recursion #45

pavel-kirienko opened this issue Aug 9, 2023 · 2 comments
Labels
help wanted Extra attention is needed
Milestone

Comments

@pavel-kirienko
Copy link
Member

pavel-kirienko commented Aug 9, 2023

Recursive functions violate MISRA C:2012 rule 17.2.

We have three recursive functions that can be found by searching for -no-recursion, all of them performing tree traversal:

  • rxFragmentDestroyTree -- simply descends the tree to deallocate every node;
  • rxSessionDestroyTree -- ditto;
  • rxSlotEjectFragment -- a complicated beast.

All of them, especially the first two, can be transformed into recursion-free alternatives but that comes at a cost of either:

  • Using an additional container for keeping temporary state during traversal. Currently, we use the call stack for that.
  • Extending each node with additional state to allow navigation through the tree during traversal.

The first option requires log2 memory and it could be implemented by asking the user for some storage area; the required size depends on the number of nodes on the network (worst case 65535 nodes) or the number of fragments in a transfer (worst case $2^{31}$ fragments). This is not a lot of memory; the maximum depth of an AVL tree is $\lceil{} 1.44 \log_2(N) - 0.328 \rceil{}$, which in our case evaluates to 45 levels.

The second option requires linear memory that will be allocated together with each item in the tree. It is linear as opposed to logarithmic because only a small fraction of the memory will be used at any given time during tree traversal. While this solution is wasteful memory-wise, it does not complicate the API because it does not require the user to provide extra storage explicitly (it is taken from the memory resource implicitly).

Another idea that expands upon the first option is to make a static thread-local stack of 45 elements defined internally within the library. This way we can implement the first option without extending the API.

Originally posted by @pavel-kirienko in #43 (comment)

@pavel-kirienko pavel-kirienko added the help wanted Extra attention is needed label Aug 9, 2023
@pavel-kirienko
Copy link
Member Author

Scott has prepared a diagram that helps visualize the issue:

image

@pavel-kirienko
Copy link
Member Author

Actually we don't even need any extra storage.

PXL_20240710_223843635 MP

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant