On 02/22/12 16:53, Brock Pytlik wrote:
On 02/22/12 14:04, Shawn Walker wrote:
On 02/21/12 17:46, Edward Pilatowicz wrote:
On Wed, Feb 15, 2012 at 09:04:03AM -0800, Shawn Walker wrote:
Greetings,
The following webrev contains general changes to improve the overall
performance of the package system:
7145683 explore general pkg performance improvements
webrev:
https://cr.opensolaris.org/action/browse/pkg/swalker/pkg-7145863/webrev/
[snip]
----------
src/tests/cli/t_pkgdep.py
- i have no idea why you removed "%(pfx)s.path=isadir/pfoo2/baz" from
double_double_stdout.
Because the test was verifying that if a user specified the same
search path twice to pkgdepend that would be seen twice in the
pkgdepend debug output. I decided it was silly for pkgdepend to
actually care that they specified it twice, and to simply ignore
redundant specifications. As a result, you'll only see one unique
entry in the output. Doing that also changed the sort order ...
[snip]
(also all changes in pkgdep)
I'd appreciate actual changes in functionality not to get rolled into a
bug dealing only with performance improvements.
This change was originally necessary because of the calls to
consolidate_attrs. It caused the pkgdep test case to fail, so it was
necessary.
Once a bug has been filed, I'm ok w/ this change in functionality,
though I'm not particularly sure why it's needed. (Perhaps that can be
included in the bug.)
As a side note, this change does actually contradict what we say in our
section 5 manpage, which is that the multiply listed attributes are
treated as unordered lists, not sets. Now, do I think it's both silly
and unlikely for someone to depend on how many times a particular
attribute name/value pair appears? Yes. But these changes do appear to
change our behavior on that issue.
I disagree. The consolidate_attrs thing was there long before I came
along to make this change. And I'm not changing the values into sets,
I'm changing them from sets into lists.
Either way, because of other changes made during the review I'm now able
to revert the changes to pkgdep.py and *some* of the changes to t_pkgdep.py.
imageplan.py:
1810-1811: a comment here about why we're doing something that appears
to be more work than is necessary would help. I assume it's because,
usually, oactions is empty and that check is so much faster than finding
the length of oactions ... but then I would've expected that finding the
length of an empty list to be rather fast itself...
That's actually leftover code from when I was originally making oactions
None instead of an empty list. But then I found too many places that
depended on it being a list. So I've reverted this.
But yes, checking for "not x" is about ~28% (?) faster than checking for
"len(x) == 0":
>>> def foo(x):
... if not x:
... return
...
>>> dis.dis(foo)
2 0 LOAD_FAST 0 (x)
3 JUMP_IF_TRUE 5 (to 11)
6 POP_TOP
3 7 LOAD_CONST 0 (None)
10 RETURN_VALUE
>> 11 POP_TOP
12 LOAD_CONST 0 (None)
15 RETURN_VALUE
>>> def bar(x):
... if len(x) == 0:
... return
>>> dis.dis(bar)
2 0 LOAD_GLOBAL 0 (len)
3 LOAD_FAST 0 (x)
6 CALL_FUNCTION 1
9 LOAD_CONST 1 (0)
12 COMPARE_OP 2 (==)
15 JUMP_IF_FALSE 5 (to 23)
18 POP_TOP
3 19 LOAD_CONST 0 (None)
22 RETURN_VALUE
>> 23 POP_TOP
24 LOAD_CONST 0 (None)
27 RETURN_VALUE
>>> n = 20000
>>> import timeit
>>> t = timeit.Timer("foo([])", "from __main__ import foo").timeit(n)
>>> t2 = timeit.Timer("bar([])", "from __main__ import bar").timeit(n)
>>> print "%20f %8d calls/sec" % (t, n / t)
0.004127 4846385 calls/sec
>>> print "%20f %8d calls/sec" % (t2, n / t2)
0.005734 3487986 calls/sec
generic.py:
599-600: I assume you also compared this with using an if/elsif
construction?
The one it was using previously? If so, see my last reply to Ed with
get_new_varcet_keys() comparisons.
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
