2008/7/1 Dan Price <[EMAIL PROTECTED]>:
>
> Hi folks,
>
> I finally have round two of my perf work ready.  Please refer to
> http://defect.opensolaris.org/bz/show_bug.cgi?id=2344#c1 for the
> full perf results, but the punchline is:
>
>                                    OLD CODE      NEW CODE    DIFF
>  ----------------------------------------------------------------------
>  pkg image-update -n               130.6 sec     103.4 sec    1.2x
>  pkg list                           88.8 sec      15.6 sec    5.6x
>  pkg list -a                       106.8 sec      32.5 sec    3.2x
>  pkg install -n sunstudio           40.4 sec      32.5 sec    1.2x
>
>
> Please look this over carefully.  I would like at least two reviewers.
>
>   http://cr.opensolaris.org/~dp/fmri-perf

http://cr.opensolaris.org/~dp/fmri-perf/src/modules/fmri.py.wdiff.html
==========

      344 +                outstr = auth[len(PREF_AUTH_PFX_):]

For pure speed reasons, since the value of PREF_AUTH_PFX_ doesn't
change, couldn't we also pre-calculate the length after line 42?

  42 PREF_AUTH_PFX_ = PREF_AUTH_PFX + "_"

And use that at line 344?

http://cr.opensolaris.org/~dp/fmri-perf/src/modules/server/transaction.py.wdiff.html
==========
  34   34  import time

This appears to be an unused import now.

       93 +                self.open_time = datetime.datetime.utcnow()

If I understand the python docs correctly, line 93 can include a
fractional amount of time. These seems to be addressed by get_basename
indirectly as it always returns a signed decimal value for the time.
However, is there any concern that we should normalise this value at
this point to not include fractional amounts so as to prevent any
possible failures elsewhere?

http://cr.opensolaris.org/~dp/fmri-perf/src/modules/version.py.wdiff.html
==========
       48 +                exceptions.Exception.__init__(self)

       89 +                exceptions.Exception.__init__(self)

"Python in a Nutshell: A Desk Reference" recommends the following
syntax instead:

        def __init__(self, *args):
                exceptions.Exception.__init__(self, *args)

In addition, a discussion from last year indicates that the Python
developers (Gudio, et al.) believe that a recommendation to not rely
on self.args is appropriate. Instead, explicitly named arguments are
the preference [2].

      182 +        def set_timestamp(self,
timestamp=datetime.datetime.utcnow()):

Missing space after and before '='.

http://cr.opensolaris.org/~dp/fmri-perf/src/tests/perf/fmribench.py.html
==========
What's with the commented code in lines 224-237?

Otherwise, your changes look fine, and they checked when I applied
your patch to my local copy.

Cheers,
-- 
Shawn Walker

[1] http://mail.python.org/pipermail/python-list/2003-April/201396.html
[2] http://mail.python.org/pipermail/python-3000/2007-July/008871.html
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to