Fix SortedSet.IsSubsetOf (and friends) 102118

[lilinus]

Fix #102118

The required array size for the bit helper BitHelper in CheckUniqueAndUnfoundElements (with this change calculated with GetInternalIndexOfBitHelperLength) grows quite rapidly.
So I added a threshold where the algorithm for GetInternalIndexOfBitHelperLength will return -1, where the implementation in CheckUniqueAndUnfoundElements switches to a fallback algorithm, which does not have same extreme memory complexity.

The fallback algorithm tries to find the node, and add it to a HashSet to remember if it has been seen before.

The algorithm used in TreeSet was not affectd by this bug, so I have not changed the implementation in this PR.

[lilinus]

Just took this value becuase it seemed "reasonable" to me according to numbers above in comments.

[lilinus]

Anyone has an idea of how to test the case when Count is above InternalIndexOfCountThreshold without using big collections?

Would it be viable to have InternalIndexOfCountThreshold be a mutable instance field we set in unit tests, if DEBUG is defined?

[danmoseley]

Can you use the big collection but conditionalize the test on 64 bit?

[sglienke]

I recently came across this bug and was thinking about this solution.

Unless I am missing something we can use the fact that up to the height/2 of the tree, we have a complete tree and only fall back to a different way of calculating the index of nodes in levels after that.

[lilinus]

I recently came across this bug and was thinking about this solution.

Unless I am missing something we can use the fact that up to the height/2 of the tree, we have a complete tree and only fall back to a different way of calculating the index of nodes in levels after that.

Hmm something like that could be used, but as soon as we are not implying a position based on a full tree, we still need to traverse a big part of the tree to simply find the number of nodes that are "left" (with a common parent node) of a specific node.

[sglienke]

That is true, and it may not even be possible - I just worry that the current implementation might degrade performance too much.

And just a small nitpick: we might confuse the terms "complete tree" and "full tree" - the following is a full but not a complete tree:

            0
      1           2
  3       4   
        5   6        

[lilinus]

That is true, and it may not even be possible - I just worry that the current implementation might degrade performance too much.

Yeah it's not an optimal algorithm, the pro is the much smaller memory footprint. Also it is already used for InternalIndexOf in
TreeSubSet.

I'm planning doing a follow-up PR to speed it up a bit, reduce allocations etc. But did not want to mess too much with any existing code since this is only a bug-fix.

Any suggestion for a better algorithm for the case when the count is above the limit is welcome.

And just a small nitpick: we might confuse the terms "complete tree" and "full tree"

Gotcha, will make sure the terms are used correctly.

[lilinus]

I have changed the fallback implementation now (and updated the description accordingly). PTAL @sglienke