BIND 10 #404: serialize RDATA for in memory data source

[BIND 10 Development]

#404: serialize RDATA for in memory data source
-------------------------------------+-------------------------------------
                 Reporter:  jinmei   |                Owner:  jinmei
                     Type:  task     |               Status:  reviewing
                 Priority:  major    |            Milestone:  y2 12 month
                Component:  data     |  milestone
  source                             |           Resolution:
                 Keywords:           |            Sensitive:  0
Estimated Number of Hours:  0.0      |  Add Hours to Ticket:  0
                Billable?:  1        |          Total Hours:  0
                Internal?:  0        |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 First, I want to say I like the approach. But large changeset deserves
 larger amount of comments ;-)

 In the benchmark, using vector<uint8> as a buffer and using casted pointer
 to &buffer[0] seems kludge. First of, the fact that it should work is not
 so apparent from the vector interface, one needs to know the guarantee
 that the elements must be stored in continuous array of memory. Second,
 sizeof returns the size in chars and the standard guarantees aliasing
 compatibility with unsigned char pointers only, not with uint8 *. So, the
 compiler is free to deduce these two pointers can never point to the same
 memory and the cast breaks that. So, if you want to use vector and not
 bare pointer to array, could you please use unsigned chars?

 In the AbstractMessageRenderer, all the writeUint8 and alike methods are
 really small inside and I can't really imagine any way they would do
 anything else than just write the data into some buffer (not counting
 writeName). The fact you made them virtual, I guess you made the call to
 them much more costly than the code inside them and made them impossible
 to inline. Isn't there a way to have only one implementation for them? The
 dividing into fields could be done in other functions, like writeName.

 Is std::vector really not portable? We use it in headers all the time. Is
 the hiding in pimpl really worth the prize of the overhead and more
 complicated implementation?

 RdataFieldComposer has many small methods. Is it worth writing them into
 the class header and then putting them outside? Isn't it nice to put their
 bodies inside the class and hint the compiler these methods are worth
 inlining (though the compiler does mostly what it wants today anyway).

 The RdataFields::toWire has a weard statement: condition ? true : false.
 Wouldn't condition (without the ?) be enough?

 Looking at the FieldSpec ‒ is it probable we use all 16 bits of the length
 field? Surely not for a name and if it is possible to have such long
 binary fields (I doubt there are any in the wild), we could split it into
 more consecutive fields. If we spared 2 bits from the length, we could put
 the type of field to the same 2-byte word (either by manually masking &
 shifting) or by field width specification (but I fear it's feature of
 C99). Since now it takes 3 bytes and another byte will be padding, because
 the uint16 can't live on odd address, this would make the size half.

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