Bart Smaalders wrote:
http://cr.opensolaris.org/~barts/CachedManifests/
Reduces working set by 2x on pkg install -nv,
50% reduction in time taken.
6904 image.installed_file_authority always tries to open installed file r+
7455 Should not need to read entire manifest to examine dependencies
Awesome!
modules/client/image.py
-----------------------
line 777: extra newline?
line 896: s/""" /"""/
line 919: s/=/ = /
line 922: spaces around braces needed
line 976: EnvironmentError instead of IOError?
lines 977-980: simplify to:
if e.errno not in (errno.EACCES, errno.EROFS):
raise
modules/client/imageplan.py
---------------------------
lines 28-31: I thought you were a fan of alphabetical imports :-)
lines 253-256: I think the formatting is supposed to be the following?
dirs = set([
"var",
"var/pkg",
"var/sadm",
"var/sadm/install"
])
modules/manifest.py
-------------------
lines 340-341: need re-wrapping since they moved?
line 345: extra newline?
line 457: having __image be a class variable seems problematic if
multiple image objects are in use by the client (especially in the face
of linked images in the future).
line 469: s/present/present./
line 509: open with "rb" ?
line 517: s/""" Part/"""Part/
line 522: s/get_default_publisher/get_preferred_publisher/ (the
former doesn't even exist in gate tip any more)
line 537: extra newline?
line 545: s/""" create/"""create/
lines 560, 567: s/"w"/"wb"/ ?
line 578: s/""" from/"""from/
line 587: s/""" create/"""create/
line 607: does 'v + f' need (), at the moment I'm not sure if it
means "blah for name in v and then concatenate f, or blah for name in (v
+ f)"
line 618: is this necessary?
line 623: s/""" return/"""return/
lines 635-639: the formatting is a little weird here
line 642: open with "rb" ?
line 649: drop four spaces?
line 650: why assign to s instead of just returning list(set(...)) ?
line 665: s/loaded :/loaded:/
line 771: extra newline?
----------
Overall, looks great.
Any way to add a test case to verify that the caching occurs (i.e. check
for existing of cache files and check their contents)?
I'm going to try out your patch locally and see if I spot anything else.
Cheers,
--
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
