Thomas Pelle Jakobsen <[EMAIL PROTECTED]> writes:

> # HG changeset patch
> # User Thomas Pelle Jakobsen <[EMAIL PROTECTED]>
> # Date 1226015530 -3600
> # Node ID 693761d8181f39ccbbf699b222d97c761127b461
> # Parent  75e5113f27777649c2001b1221c9717d8d375423
> Added basic benchmark class.
>
> diff -r 75e5113f2777 -r 693761d8181f apps/benchmark/benchmark.py
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/apps/benchmark/benchmark.py     Fri Nov 07 00:52:10 2008 +0100
> @@ -0,0 +1,176 @@
> +# -*- coding: utf-8 -*-
> +#
> +# Copyright 2007, 2008 VIFF Development Team.
> +#
> +# This file is part of VIFF, the Virtual Ideal Functionality Framework.
> +#
> +# VIFF is free software: you can redistribute it and/or modify it
> +# under the terms of the GNU Lesser General Public License (LGPL) as
> +# published by the Free Software Foundation, either version 3 of the
> +# License, or (at your option) any later version.
> +#
> +# VIFF is distributed in the hope that it will be useful, but WITHOUT
> +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> +# or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General
> +# Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with VIFF. If not, see <http://www.gnu.org/licenses/>.
> +
> +import time
> +import copy
> +import os
> +import sys
> +import subprocess
> +import re
> +from database import Database
> +from util import exec_on_host
> +
> +        
> +def parse_args(args):

Is this not just like the parse function I found in the patch before?

> +    """ Creates a dict from a list of strings a la k1=val1. """
> +    res = {}
> +    for a in args[1:]:
> +        # Note: This allows attribute names to contain '='
> +        s = a.rsplit("=",1)
> +        # TODO: Info about attribute types should be included. Here, we
> +        #       simply treat an attribute as int if possible and otherwise
> +        #       as a string.
> +        try:
> +            res[s[0]] = int(s[1])
> +        except ValueError:
> +            res[s[0]] = s[1]
> +    return res
> +
> +class Benchmark:
> +    """Extend this class and override its benchmark method with code that
> +    produces benchmark results, you 

Cut-off sentence?

> +    The benchmark is run on an arbitrary host from the list of hosts that is
> +    supplied when creating the Suite.
> +    
> +    These are reserved attributes that shouldn't be overridden or removed:
> +
> +            benchmark_id:
> +                Internal database id for the current benchmark.
> +            host_id:
> +                Internal database id for the current host.
> +            db_host, db_port, db_name, db_user, db_passwd:
> +                Credentials and info for the database. Same as was given to 
> the
> +                database passed to the suite constructor.
> +
> +        and these must always be set manually:
> +
> +            runs:
> +                Number of times to repeat the benchmark.
> +
> +        and additionally, when extending VIFFBenchmark,
> +
> +            n:
> +                The number of players that should participate in current 
> +                benchmark.    
> +            threshold: The security threshold for the current benchmar.
> +            player_id:
> +                Unique non-zero integer assigned to the current host.
> +            use_ssl:
> +                This is True by default, but it should still not be removed 
> or
> +                overwritten.
> +
> +    """
> +    def __init__(self, attr):
> +        self.attr = attr
> +        
> +    def attribute(self, name, val):
> +        self.attr[name] = val

Hmm, this is not Java :-) You might want to look at this nice attrdict
class: (first patch hunk)

  http://www.bitbucket.org/mg/hgchart/changeset/01bd2a3a32ea/

It allows you to use self.attr.db_host instead of self.attr['db_host'],
and if you let the Benchmark class inherit from attrdict you could skip
the 'attr' layer.

> +    def report_result(self, result):
> +        print "Reporting to:"
> +        print " db_host=" + self.attr['db_host']
> +        print " db_name=" + self.attr['db_name']
> +        print " db_port=" + str(self.attr['db_port'])
> +        print " db_user=" + self.attr['db_user']

The print statement allows you to write

  print " foo:", foo
  print " bar:", bar

and it will automatically insert a space.

> +        print "Result:"
> +        print str(result)
> +        database = Database(host=self.attr['db_host'],
> +                    db=self.attr['db_name'],
> +                    user=self.attr['db_user'],
> +                    passwd=self.attr['db_passwd'],
> +                    port=self.attr['db_port'])
> +        database.report_result(self.attr['benchmark_id'], 
> self.attr['host_id'],
> +                               result)
> +    
> +    # This method is invoked once at each host
> +    # - order: ?
> +    def setup(self, attr):
> +        print "Setting up Benchmark"
> +
> +    # This method is invoked only once for a benchmark
> +    # - on the host that starts it all
> +    def setup_once(self, suite):
> +        print "Setup once in Benchmark"
> +        # TODO: Hack -> Benchmarks must then exist only in files with same 
> name
> +        filename = str(self.__class__).split('.')[-1]
> +        if sys.platform == 'win32':
> +            scp = "pscp"
> +        else:
> +            scp = "scp"
> +        # Note that we use the benchmark classes from the repository in which
> +        # the suite is run. It would be confusing to use the classes in the
> +        # revisions that are checked out for benchmarking.
> +        #
> +        # TODO: However, I'm not sure whether generate-config-files and
> +        # generate-certificates from the "master" repository or from the
> +        # benchmark repositories should be used?!
> +        #
> +        hostname = suite.host_name.values()[0] # Execute this on arbitrary 
> host
> +        
> +        # TODO: This is hacky. Not sure whether to copy these or to
> +        # supply work_dir and assume that they are present at work_dir:
> +        # Currently, only benchmarks in the example folder is copied.
> +        # Would be better to get the absolute path for a class from python.
> +        subprocess.call([scp, "benchmark.py", suite.user + "@" + hostname +
> +                         ":" + suite.work_dir])

When building strings, the Pythonic way is to do

  "[EMAIL PROTECTED]:%s" % (suite.user, hostname, suite.work_dir)

which created fewer intermediate str objects and gives you more ways to
format the inserted strings (not that you need that here).

> +        subprocess.call([scp, "database.py", suite.user + "@" + hostname +
> +                         ":" + suite.work_dir])
> +        subprocess.call([scp, "suite.py", suite.user + "@" + hostname +
> +                         ":" + suite.work_dir])
> +        subprocess.call([scp, "util.py", suite.user + "@" + hostname +
> +                         ":" + suite.work_dir])
> +        subprocess.call([scp, "examples/" + filename + ".py", suite.user + 
> "@" +
> +                         hostname + ":" + suite.work_dir])

Who creates the "examples/" folder on the receiving side? I guess the
copy fails otherwise.

> +    def teardown(self, attr):
> +        # Remove config files, etc.
> +        print "Tearing down Benchmark"
> +    
> +    # This is the part that actually is to be benchmarked
> +    # Return: Dict containing attribute->value pairs

The type is called 'dict', always with small 'd'.

> +    def benchmark(self, attr):
> +        raise NotImplemented("Override this abstract method in a subclass.")
> +
> +    def run_on_master(self, suite):
> +        self.setup_once(suite)
> +        # TODO: Hack -> Benchmarks must then exist only in files with same 
> name
> +        filename = str(self.__class__).split('.')[-1] + ".py"
> +        # TODO: Would like to run ./benchmarkname.py instead, but that 
> +        # doesn't work on Windows.
> +        host_id = suite.host_name.keys()[0] # Use arbitrary host.
> +        command = "cd %s; python -u %s" % (suite.work_dir, filename)

Why is '-u' needed when executing Python?

> +        command = command + " host_id=%d" % host_id
> +        for attr, val in self.attr.items():
> +            command = command + " " + attr + "=" + str(val)

This applied to the immutable strings in Java too: this could require
copying O(n^2) bytes. The problem is the intermediate copies created in
most Python versions:

  http://wiki.python.org/moin/PythonSpeed/PerformanceTips#StringConcatenation

> +        exec_on_host(suite.user, suite.host_name[host_id], [command], 
> sync=True)
> +    
> +    def run_on_slave(self):
> +        self.setup(self.attr)
> +        # TODO: Might also write to text file and parse - more robust?
> +        results = []
> +        for run in range(int(self.attr['runs'])):
> +            
> +            print "**** Run %d" % run
> +            res = self.benchmark(self.attr)
> +            print "**** Result: " + str(res)
> +            results.append(res)
> +        self.report_result(results)
> +        self.teardown(self.attr)
> +        sys.exit(0)
> diff -r 75e5113f2777 -r 693761d8181f apps/benchmark/util.py
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/apps/benchmark/util.py  Fri Nov 07 00:52:10 2008 +0100
> @@ -0,0 +1,68 @@
> +# -*- coding: utf-8 -*-

[...]

> +# TODO: Some clean-up nessecary, e.g. on remote host?
> +#
> +def exec_on_host(user, host, commands, sync=True, verbose=True):
> +    """If mode = sync, the method blocks until command has finished and 
> returns
> +    none (throws exception if errorcode != 0). If mode = async, a
> +    subprocess.Popen object, p, containing the running command is returned.
> +    """
> +    if verbose:
> +        print "Starting command on " + user + "@" + host + ":"
> +        print ''.join([command.strip() + " " for command in commands])

This should be the same and will not end with a trailing space:

        print ' '.join([command.strip() for command in commands])

you could even write it in functional style:

        print ' '.join(map(str.strip, commands))

> +    if sys.platform == 'win32':
> +        cmd = ["plink", "-x", "-a"] + [user + "@" + host] + commands
> +    else:
> +        cmd = ["ssh", "-x", "-a"] + [user + "@" + host] + commands

This kind of platform dependent code is scary... I'm even considering
using the SSH client in Twisted instead to get something platform
independent...


-- 
Martin Geisler

VIFF (Virtual Ideal Functionality Framework) brings easy and efficient
SMPC (Secure Multiparty Computation) to Python. See: http://viff.dk/.

Attachment: pgpjO7Dm7KvkK.pgp
Description: PGP signature

_______________________________________________
viff-patches mailing list
[email protected]
http://lists.viff.dk/listinfo.cgi/viff-patches-viff.dk

Reply via email to