Thomas Pelle Jakobsen <[EMAIL PROTECTED]> writes:

Hi Thomas,

I've just had a quick look through the code and wow, it looks very
impressive!

But are the patches not ordered in the wrong order? I mean, the first
patch imports code which is only introduced in patch 5.

> Here's my initial attempt at automating benchmarks and graph
> generation in VIFF. Some parts of it is borrowed from the
> benchmark.py script that's alread in the repository.
>
> The main goal is to make it easy to write, run, and generate graphs
> for distributed VIFF benchmarks while at the same time not limiting
> the kind of benchmarks that could be made. Another design goal is
> that the code should scale as the number of benchmarks increase. An
> approah is chosen where benchmark data is collected in a central
> database rather than as a bunch of text files. My feeling is that
> this eases data maintainability and production of more complex
> statistics.

Sure -- I have never believed that tons of text files were the best
way to store long term benchmark data. It was simply the easiest and
fastest way for me to collect data.

> The code is by no means finished. Lots of TODO's are still there,
> documentation can be improved, and I haven't done much to remove
> trailing whitespaces, etc. However, I've reached a state where I'm
> actually able to use it to run benchmarks on the DAIMI hosts while
> data is reported to a MySQL database at my computer at home.

Cool! I saw you write somewhere that an ORM would be nice and I agree.
After having used such an interface I cringe when I see the raw SQL
strings in the code -- there are just so many possibilities for
putting MySQL specific stuff there, not to talk about the possible
robustness and security problems.

But we should not make that stop us from starting out like this.

> I post the patch now hoping that an early review will result in time
> saved, so please feel free to comment on the code. I'd be happy to
> hear about bugs and ideas for improving it. I'm quite a newbie in
> Python, so if I've done something very non-pythonic, please let me
> know, too :-)

I saw that you're using strings here and there as big comments:

  """bla bla, my nice
  explaination for
  something complex"""

instead of the more natural

  # bla bla, my nice
  # explaination for
  # something complex

It is only the very first string literal that is treated as a
doccomment in Python -- the others are just string literals.

Also, I saw you do

  if 'foo' in some_dict.keys():

which is the same as

  if 'foo' in some_dict

When you do

  str(benchmark.__class__).split('.')[-1]

you could instead do

  benchmark.__class__.__name__

which is still sort of a hack, but is easier to understand.

Lastly, since the files are introduced this year, the copyright
statements should simply read

  # Copyright 2008 VIFF Development Team.


I hope I'll have time to look at and play with the code some more.
Mikkel, Jakob, Sigurd, ...: please jump in and let me/Thomas know what
you think of the code and design!

-- 
Martin Geisler

VIFF (Virtual Ideal Functionality Framework) brings easy and efficient
SMPC (Secure Multiparty Computation) to Python. See: http://viff.dk/.
_______________________________________________
viff-patches mailing list
[email protected]
http://lists.viff.dk/listinfo.cgi/viff-patches-viff.dk

Reply via email to