John Rice wrote: > Great that looks simpler again - we'll make the changes when are back in > on Tuesday, test to make sure this does what's needed for both the GUI > clients and respin. If Danek hasn't found a fix for the __init__.py > issues we'll revert this file to r590. > > Bug logged to track root cause analysis of problem post 2008.11 (4116 > depends on this): > 4231:Resolve root cause for memory leak and remove workaround for 4116 > > Brock you happy with this approach, as opposed to modifying the API? > Actually, I find modifying the api cleaner than this. This just makes my skin crawl personally. I'm not inherently opposed to leaving the API workaround in for a while we get things fixed up. I think an API option of "prefer low memory use over blazing speed" is at least a possibly reasonable switch to provide. Hard coding in our pkg client name here seems wrong to me on many levels. If this is what everyone else likes, I'm fine with it for 2008.11, but this workaround I want out of the code as soon as possible.
Brock > JR > > Shawn Walker wrote: > >> jmr wrote: >> >>> Michal has respun his webrev without the actions:__init__.py revert >>> to r590 as well, to give us both options. >>> >>> With __init__.py from the trunk: >>> http://cr.opensolaris.org/~migi/23_10_2008_bug_4116_v1_no_init/ >>> >>> With __init__.py from rev590: >>> http://cr.opensolaris.org/~migi/23_10_2008_bug_4116_v1/ >>> >> Earlier today, it occurred to me that we know which pkg client is >> currently using the API via history. As such, rather than add a >> temporary flag to the api to control cache usage, unintentionally >> exposing it, it makes more sense to just limit the caching to the cli >> as a temporary workaround instead. >> >> To do this, I would remove all of the use_cache changes you've made >> and make the following simple changes to modules/client/image.py: >> >> diff -r c95448e970e5 src/modules/client/image.py >> --- a/src/modules/client/image.py Fri Oct 24 15:36:26 2008 -0500 >> +++ b/src/modules/client/image.py Fri Oct 24 15:42:28 2008 -0500 >> @@ -935,11 +935,14 @@ >> """Find on-disk manifest and create in-memory Manifest >> object, applying appropriate filters as needed.""" >> >> - if fmri in self.__manifest_cache: >> - m = self.__manifest_cache[fmri] >> + if self.history.client_name == "pkg": >> + if fmri in self.__manifest_cache: >> + m = self.__manifest_cache[fmri] >> + else: >> + m = self.__get_manifest(fmri) >> + self.__manifest_cache[fmri] = m >> else: >> m = self.__get_manifest(fmri) >> - self.__manifest_cache[fmri] = m >> >> self.__touch_manifest(fmri) >> >> >> ========================== >> >> The above change is only intended to be temporary, and a bug would >> need to be filed to address the real issue. However, this to me, is >> far simpler and preferable to adding a use_cache flag which also bumps >> the API version and so on. >> >> Code Review follows: >> src/packagemanager.py: >> line 40: s/lenght/length/ >> >> lines 580, 992-994: Suggested word change s/Fetching/Retrieving/ >> >> line 1112: tue users? what's that? >> >> Otherwise, looks fine. >> >> Cheers, >> > > _______________________________________________ > pkg-discuss mailing list > [email protected] > http://mail.opensolaris.org/mailman/listinfo/pkg-discuss > _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
