BIND 10 #327: early work on recursion

[BIND 10 Development]

#327: early work on recursion
---------------------------+------------------------------------------------
      Reporter:  each      |        Owner:  each                 
          Type:  defect    |       Status:  new                  
      Priority:  major     |    Milestone:  y2 12 month milestone
     Component:  recurser  |   Resolution:                       
      Keywords:            |    Sensitive:  0                    
Estimatedhours:  0.0       |        Hours:  0                    
      Billable:  1         |   Totalhours:  0                    
      Internal:  0         |  
---------------------------+------------------------------------------------

Comment(by stephen):

 Branch created: r2876
 Code reviewed: r2941

 As the request is for a look at what has been done so far, I've
 concentrated on the restructing using the stackless coroutines rather than
 checking the code is functionally correct.

 '''src/bin/auth/yield.h'''
 '''src/bin/auth/unyield.h'''
 '''src/bin/auth/coroutine.h'''
 These are files written by Christopher M. Kohlhoff and released under the
 Boost Software License V1.0.

 Regarding the use of the macros, the code generated by them is  convoluted
 - jumps into the middle of "if" blocks and "for" loops.  I only got to
 understand it by writing a simple program, expanding it with the
 preprocessor, formatting the output, then stepping through it with gdb.
 However, they do appear to work.

 '''src/bin/auth/asio_link.h'''
 As the various constructors for DNSProvider and !CheckinProvider are
 empty, you could declare the body of the function in the header file - it
 would allow for some optimisation.

 Also, are default definitions for the operator() methods in these classes
 needed?  If not (and I guess not as the documentation suggests that the
 classes are supposed to be abstract) and if this method overridden by a
 derived class, just declare the method abstract (along the lines of "void
 operator() const = 0" in the class definition).  It will save a few bytes
 of generated code.


 '''src/bin/auth/asio_link.cc'''

 ''Class TCPServer''
 Should the checkin_callback_ and dns_callback_ object also be encapsulated
 in a boost::shared_ptr, as they must persist across calls to the
 operator() method.

 Regarding the "fork" call in operator(), I'm not clear how the scheduling
 works, especially if multiple TCPServer children are created.  Does a
 child run to completion before the parent?  If not, what happens when a
 second child is created - is the previous object now unreferencable?  Some
 detailed comments are required.

 ''Class UDPServer''
 I didn't find the "yield continue" construct in the "Thinking
 Asynchronously in C++" article.  What does it do? (Given the nested loops
 within the created by the macro expansions, is there a danger that it will
 cause control flow to transfer to an unexpected point in the code if used
 in the wrong place?)

 '''src/bin/auth/auth_srv.h'''
 OK

 '''src/bin/auth/auth_srv.cc'''
 The getCheckinProvider() and getDNSProvider() methods are sufficiently
 trivial that they could be placed in the header file.

 '''src/bin/auth/tests/asio_link_unittest.cc'''
 I've not really checked this in detail - most of the changes seem to do
 with the new signature for the IOService constructor.

 '''src/bin/auth/main.cc'''
 OK

 '''Summary'''
 The use of reenter/yield/throw does seem to produce code that is more easy
 to follow that splitting the processing sequence into multiple functions,
 in that the processing sequence is all in one place.  But since this is
 something new (and, as noted above, the generated code is convoluted and
 the scheduling of coroutine calls possibly quite complex), there is the
 question as to whether this comes in the area of "too complex" (as we
 discussed at the last BIND-10 meeting).  This is perhaps something that
 should be raised at the next conference call.

 Regardless of the  I think that there should be some significant detailed
 of what the macros do, either in in the code or in an associated README
 file.  (A reference to a web site is insufficient as it may disappear at
 any time in the future.)

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