BIND 10 #241: review: benchmark framework

[BIND 10 Development]

#241: review: benchmark framework
-------------------------------+--------------------------------------------
      Reporter:  jinmei        |        Owner:  jinmei                     
          Type:  enhancement   |       Status:  reviewing                  
      Priority:  major         |    Milestone:  06. 4th Incremental Release
     Component:  Unclassified  |   Resolution:                             
      Keywords:                |    Sensitive:  0                          
Estimatedhours:                |        Hours:                             
      Billable:  1             |   Totalhours:                             
      Internal:  0             |  
-------------------------------+--------------------------------------------
Changes (by jelte):

  * owner:  UnAssigned => jinmei


Comment:

 Had a discussion on jabber about the changes; summary is that I see no
 reason not to merge it as it is, with the reservations i had being added
 to the documentation as future work.

 --

 (07:06:57 PM) jelte: i was wondering, should we have separate directories
 like we do for normal unittests?
 (07:08:11 PM) jelte: the thing being that benchmark_util contains mostly
 libdns++-specific stuff :)
 (07:09:23 PM) jelte: also i was thinking if we could make it more like
 gtest (in that we wouldn't need to define main(), and for trivial cases
 not even subclass something, but have macros), but that could perhaps be
 added later, in general i think the code looks ok afaict
 (07:09:25 PM) jinmei: as for separate directories I don't have a strong
 opinion, but the initial idea was to make separate benchmark directories
 for different modules
 (07:09:37 PM) jinmei: e.g. src/lib/dns/benchmarks
 (07:09:37 PM) jelte: right, i thought so
 (07:09:56 PM) jinmei: I'm not sure if the second point is related to the
 directory issue
 (07:09:59 PM) jelte: making the benchmark dir not much more than that
 header file :)
 (07:10:02 PM) jelte: no it isn't :)
 (07:10:25 PM) jinmei: I
 (07:10:31 PM) jinmei: oops mistyped
 (07:10:37 PM) jelte: but it occurred to me while i was reading through the
 example again :)
 (07:11:35 PM) jinmei: as for the second point, I know that.
 (07:12:32 PM) jinmei: maybe we'll have other benchmark utils that are not
 so specific to DNS here.
 (07:13:00 PM) jinmei: e.g. we may want to do some network I/O specific
 benchmarks and we want to have utilities for that
 (07:13:20 PM) jinmei: but honestly I don't know how it will evolve
 (07:14:00 PM) jelte: well if we keep the framework separate, and decide on
 lib/dns/benchmarks/ than what's currently in benchmark_util.* could go
 there i think
 (07:14:27 PM) jelte: yeah that was my other thing, it looks ok to me right
 now, but once we start writing other benchmarks we're bound to run into
 things :)
 (07:15:18 PM) jinmei: ah
 (07:15:46 PM) jinmei: first off, the current benchmark_util is actually
 not intended to be used for libdns++.
 (07:16:05 PM) jinmei: the intended primary intended user is the auth
 server
 (07:16:26 PM) jinmei: with clarifying this,
 (07:16:45 PM) jinmei: the question is whether this should better be placed
 in src/bin/auth/benchmarks for example.
 (07:16:52 PM) jelte: right
 (07:17:15 PM) jinmei: again, I don't have a strong opinion,
 (07:17:56 PM) jinmei: but it's quite likely we'd want to have something
 like this for the caching (recursive) server.
 (07:18:09 PM) jelte: noting that some of our unit tests are still not
 living in their own directory :)
 (07:18:31 PM) jinmei: yeah, I've been aware of that
 (07:19:15 PM) jinmei: as for making it gtest-like, yes, that makes sense
 (07:19:59 PM) jinmei: and I sort of tried to achieve that type of goal in
 the initial design/implementation,
 (07:20:08 PM) jinmei: and I also admit this could be improved
 (07:21:32 PM) jelte: it does not have to stop us from including this as it
 is
 (07:22:23 PM) jinmei: so, for now, I'd add some description to the doxygen
 comments:
 (07:22:54 PM) jelte: oh and a more code-specific issue; benchmark.h can
 use some empty lines :)
 (07:23:03 PM) jinmei: - the design of benchmark.h may be changed so that
 it will be set up more easily.
 (07:23:42 PM) jelte: every function right now is immediately followed by
 the doxygen for the next one, which makes reading through it a bit harder
 than it needs to be
 (07:23:43 PM) jinmei: - benchmark_util may be too specific to be placed
 here, and we'll see if it should stay here or move to more appropriate
 benchmark cases.
 (07:23:54 PM) jinmei: okay
 (07:24:01 PM) jinmei: > empty lines
 (07:24:51 PM) jinmei: maybe we should add this to coding guideline?
 (07:25:00 PM) jinmei: or is it too minor?
 (07:26:25 PM) jelte: perhaps we should, i have also been reprimanded by
 michael for not using enough vertical space at one point :)
 (07:27:43 PM) jinmei: blank lines added (r2660)
 (07:29:17 PM) jelte: much better :)
 (07:39:13 PM) fujiwara entered the room.
 (07:40:51 PM) jreed: "style fix: added blank lines between blocks of
 (function + its description)" sounds good for style guide too.

 (07:41:39 PM) jreed: not sure if it is important for following function,
 but for a gap before previous lines.
 (07:45:17 PM) jinmei: added a note about future possible extension to the
 benchmark (i.e. making it gtest-like).
 (07:45:19 PM) jinmei: r2661
 (07:48:21 PM) jelte: looks good

-- 
Ticket URL: <http://bind10.isc.org/ticket/241#comment:4>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development