BIND 10 #2090: use offset_ptr in RBNode

[BIND 10 Development]

#2090: use offset_ptr in RBNode
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20120731
  medium                             |            Resolution:
                  Component:  data   |             Sensitive:  0
  source                             |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  5
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
  scalable inmemory                  |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 I made a few small changes.  Some of them was crucial for me:

 {{{#!diff
 -    if (current->*right != RBNode<T>::NULL_NODE()) {
 +    if ((current->*right).get() != RBNode<T>::NULL_NODE()) {
 }}}

 {{{#!diff
 -        while (left_most->left_ != NULLNODE) {
 +        while (left_most->getLeft() != NULLNODE) {
 }}}

 In my environment it doesn't seem to be possible to compare an
 offset_ptr and a raw pointer with !=.  Maybe it's because of a boost
 version, of because which one is performed first by the compiler, type
 conversion or evaluating the operator.

 Others are purely editorial/style matters.

 I have one small request: it won't be very obvious why we do things
 like this:
 {{{#!diff
                  node = (node_path.last_comparison_.getOrder() < 0) ?
 -                    node->left_ : node->right_;
 +                    node->getLeft() : node->getRight();
 }}}
 instead of just defining `node` as an offset_pointer and just
 substitute it with `left_` or `right_`, which would be more intuitive.
 Basically the short answer is performance so we can minimize the
 instantiation of offset_ptr objects and invoking methods on them.
 Benchmarks indeed showed these overhead can be substantial.  So, in
 general, we use raw pointers when we don't have to modify the links
 pointed by the pointers, and getXXX() methods are helper for that.
 I'd clarify these points around here:

 {{{#!cpp
     /// \name Data to maintain the rbtree structure.
 }}}

 With this, please feel free to merge.  If you want me to review it,
 I'll do it; otherwise, just commit and merge it.  I'll take a look at
 it and if I want to update it I'll piggy back it on some other tasks
 that update this file.

-- 
Ticket URL: <https://bind10.isc.org/ticket/2090#comment:8>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development