Binary tree stack overflow

Question

I have made a binary tree based on Alex Allain's example found here. It throws a stack overflow exception after adding about 5000-6000 elements to it. Any idea of how to prevent a stack overflow? The cause is that Insert() calls itself recursivly.

Update 3/6/2013

Here is how I refactored the code to avoid stack overflow:

void Insert(Key_T key, Value_T val, QuickMapNode<Key_T, Value_T> *leaf)
{
    while (true)
        if(key < leaf->key)
        {
            if(leaf->left) leaf = leaf->left;
            else
            {
                leaf->left = new QuickMapNode<Key_T, Value_T>;
                leaf->left->key = key;
                leaf->left->val = val;
                leaf->left->parent = leaf;
                leaf->left->left = NULL;    // Sets the left child of the child node to null
                leaf->left->right = NULL;   // Sets the right child of the child node to null
                break;
            }  
        }
        else if (key >= leaf->key)
        {
            if(leaf->right) leaf = leaf->right;
            else
            {
                leaf->right = new QuickMapNode<Key_T, Value_T>;
                leaf->right->key = key;
                leaf->right->val = val;
                leaf->right->parent = leaf;
                leaf->right->left = NULL;  // Sets the left child of the child node to null
                leaf->right->right = NULL; // Sets the right child of the child node to null
                break;
            }
        }
}

I think you may get stack overflow based on the input pattern. Say if the input is already sorted, then it becomes `skew tree` which causes the recursive stack depth of 6000 which is really huge. — sundar, Mar 05 '13 at 19:51
Add a function to keep track of the max depth of the tree. It should be just over 12 for a well balanced tree. — andre, Mar 05 '13 at 19:52
An easy way to "balance" the tree without resorting to a self-balancing tree is to shuffle the input items before inserting them. This might solve the problem if it is due to the tree being unbalanced. — Tom, Mar 05 '13 at 19:55
Another easy way is simply track the min/max height of each operation, and when it gets more than 4x over optimal (12x4 ~ 48), then move all nodes to a new tree in a perfectly balanced way, and then swap. It's far easier than balancing on the fly, if a lot more "jerky" — Mooing Duck, Mar 05 '13 at 20:24

ahans · Answer 1 · 2013-03-05T22:16:36.457

Like Öö Tiib said, you should change insert to be not recursive. Every recursive function can be turned into a non-recursive one by storing the data that would go on the stack in some other data structure. This way you can use the heap for those data and don't have the overhead of function calls on the stack (return address etc.) You can often use a vector or list that you use like a stack: take (and pop) the back() of the vector to get the current argument, and in places your current code would recursively call itself, you push_back() what you would have passed to the recursive function call.

Here's the insert() method from your link as an iterative version:

void btree::insert(int key, node *leaf)
{
  std::list<node*> leafs;
  leafs.push_back(leaf);

  while (leafs.size() > 0)
  {
    leaf = leafs.back();
    leafs.pop_back();
    if(key < leaf->key_value)
    {
      if(leaf->left!=NULL)
        leafs.push_back(leaf->left);
      else
      {
        leaf->left=new node;
        leaf->left->key_value=key;
        leaf->left->left=NULL;    //Sets the left child of the child node to null
        leaf->left->right=NULL;   //Sets the right child of the child node to null
      }  
    }
    else if(key>=leaf->key_value)
    {
      if(leaf->right!=NULL)
        leafs.push_back(leaf->right);
      else
      {
        leaf->right=new node;
        leaf->right->key_value=key;
        leaf->right->left=NULL;  //Sets the left child of the child node to null
        leaf->right->right=NULL; //Sets the right child of the child node to null
      }
    }
  }
}

I ran the code and it seems to work. It's much slower than the recursive version though, not sure why that is ... Both versions work fine with 10000 and more elements, so there's perhaps something else wrong in your implementation ...

Actually, when traversing a binary tree like we do here, there's no need to store any previous information, as we don't do any backtracking. Once the location for the new element is found, we're finished. So we can get rid of the list/vector altogether:

void btree::insert(int key, node *leaf)
{
  while (leaf != NULL)
  {
    if(key < leaf->key_value)
    {
      if(leaf->left!=NULL)
        leaf = leaf->left;
      else
      {
        leaf->left=new node;
        leaf->left->key_value=key;
        leaf->left->left=NULL;    //Sets the left child of the child node to null
        leaf->left->right=NULL;   //Sets the right child of the child node to null
        return;
      }  
    }
    else if(key>=leaf->key_value)
    {
      if(leaf->right!=NULL)
        leaf = leaf->right;
      else
      {
        leaf->right=new node;
        leaf->right->key_value=key;
        leaf->right->left=NULL;  //Sets the left child of the child node to null
        leaf->right->right=NULL; //Sets the right child of the child node to null
        return;
      }
    }
  }
}

You'll also get better performance from a self balancing tree. My favourite is the AA tree. — hookenz, Mar 05 '13 at 21:59

Öö Tiib · Accepted Answer · 2013-03-05T20:01:22.377

1

Make algorithm of insert that is not recursive. You do need only to search place where to insert so you do not need stack of calls for that.

edited Mar 05 '13 at 20:01

answered Mar 05 '13 at 19:53

Öö Tiib

10,809
25
44

And how would I do that? Do you have an example or a binary tree that do not use recursive calls? Thank you. – Mar 05 '13 at 19:54

score 1 · Answer 3 · answered Mar 05 '13 at 20:04

Taking guesses due to lack of provided details: assume worst case, that after 6000 inserts the stack depth is all 6000 recursive calls. Assume a reasonable stack frame size of maybe 20 bytes - then stack size is 6000 * 20 = 120,000 bytes. If the stack frame is actually 160 bytes (8x larger) then the stak size is 6000 * 160, slightly less than 1MB. I wonder ... is there a limit on your number of elements? What is the allocated stack size?
The comments above tell you how to actually solve the problem (balanced tree). I might add, just about any recursive algorithm can be converted to an iterative algorithm - it isn't as elegant and it will take effort to get it right, but then you won't fill up the stack. But if there really is (not just you think there is) a limit on the number of input elements, then it seems to me you could determine the size of the stack frame for insert and make the stack size large enough to fit #elements * stack frame size, the worst case, plus a little more for whatever extra is on the stack.

score 0 · Answer 4 · edited May 23 '17 at 12:01

0

As mentioned above the most likely cause is the stack overflow due to too many recursive insert calls

The following are the different options

Use a self balancing tree http://en.wikipedia.org/wiki/Self-balancing_binary_search_tree
Use a non recursive tree decent algorithmn.Lookk at this example Non-recursive add function in a binary tree using c++

edited May 23 '17 at 12:01

Community

1
1

answered Mar 05 '13 at 20:00

Pradheep

3,553
1
27
35

Binary tree stack overflow

4 Answers4