Michal Pryc wrote:
Brock,

My comments inline.

I've made new webrev after pylinting the code and also clearing the list
of selected packages after the preferred repository was changed:
http://cr.opensolaris.org/~migi/lists_split_11_02_2009

Packagemanager.py:
Lines 1314-1320: don't these lines mean that if I have chosen a repo
other then my default repo, select some packages from that repo which
are also present in my default repo, then click install, I'll do the
install from my default repo anyway? Since only the pkg stem is being
passed in, I'm not sure how the authority info could be passed along.
Does this also mean I can't select 3 pkgs from contrib and 2 from dev
and have them all installed in one shot?
I think I didn't fully answer this question. So if the user selects exactly the same package, which is present in many repositories the GUI will behave like CLI - the last given to the command line / the last selected will be installed.

best
Michal

The webrev introduces control model containing all selections which user
did for any of the repositories. The packages from the control model are
passed to the api, so the user is able to select many packages from many
repositories and hit proper action button to proceed with all selected
packages not only from the given (preferred) repository. As discussed
with xDesign if the user mouse over the install/update or remove button,
tooltip will appear with information how many packages and from which
repository were selected (counted separately for install/update and
remove buttons).

The model is a dictionary:

self.selected_pkgs = {}

The structure of this dictionary is as follows:

{
"auth_1_name" : [
{package_stem:package_status_enum},
{package_stem2:package_status_enum},
...
],
"auth_2_name" : [
{package_stem:package_status_enum},
{package_stem2:package_status_enum},
...
]
...
}

Two private functions are modifying this model:
__add_pkg_stem_to_list(stem, status)
This function is adding package stem to the currently selected in the
combobox authority.

__remove_pkg_stem_from_list(stem)
This function removes stem from any authority, as we want to remove from
not currently visible as well.

So the control model does know from which repository selections were
made and this is used to preserve selections when the user switch to
different repository and switch back.
Those selections are also cleaned if the user changes the preferred
repository as the stem of the package is changed. Also if the user
disables one of the repositories we are removing selections from the
control model for that repository.


I don't totally grok all the other changes, but those were the things
that jumped out at me.

Also, from bug 6365, it sounds like this is going to be a temporary fix
to get find-as-you-type working till something else which scales is
found? (If that's the case, I'm not sure that saying this fixes 6365 and
closing it makes a lot of sense unless you're planning to open another
to track the future scaling issues.) I also have some ideas on how we
might pull a feature like this out of the search API, if you're
interested let me know soon as that wad's starting to come together, and
if that's going to be something we need, I want to make sure the design
I have in mind at least allows for it going forward.
It is not a solution for bug 6365, but the split of the repositories
will for sure improve performance, that is why I've wrote "partial fix"
and for sure I will not close the bug, just update with all relevant
information. John is also introducing webrev with preference dialog
which allow to change type-in search to after-enter-pressed search,
which will address some of the problems described in 6365.


Other than 6365, I don't get why 6516 is needed, or even desired. Could
you please fill out that bug with more explanation? Is it to fix 6227?
If so, I don't think I understand how that helps.

It is not a solution for bug 6365, but the split of the repositories
will for sure improve performance, that is why I've wrote "partial fix"
and for sure I will not close the bug, just update with all relevant
information. John is also introducing webrev with preference dialog
which allow to change type-in search to after-enter-pressed search,
which will address some of the problems described in 6365.
6227 was caused by refiltering the data model too many times while the
repository was changed. The fix for this was to use self.in_setup
variable in proper places, so the refiltering is done only when needed.


Also, in the bug reports, I'm seeing a lot of reference to caching, but
I don't really understand/know what that means in this context. I
apologize if I've missed or forgotten the relevant discussions, but
could you give an outline of what caching you have in mind? It might
help me put these other changes in context.
6516 is needed as a part of more changes coming - caching. The caching
will use cpickle to dump gtk.ListStore and read it back for the given
repositories. I already have working solution, but needs some tweaking
before I'll present webrev. The working solution allows to start PM in
around 3 seconds comparing to 24 seconds when not using it (on my box
with few repos configured). Of course caching need to be a little smart
on when it is out of sync with the current catalog and status of the
packages. If the call to function __is_cache_in_sync(repository) will
return True then we will proceed with reading cpickle list, if not then
we will proceed with current flow and once it's done we will dump the
cache for that repository.

all the best
Michal Pryc


Thanks,
Brock

Michal Pryc wrote:
Hello,
The webrev is:
http://cr.opensolaris.org/~migi/lists_split_10_02_2009_v1

This change makes the per repository underlying data model allowing
selecting packages in many of them at the same time.
Before the change, the main data model contaied all packages and it's
selections. Current implementation splits that, so the selections
are stored in dictionaries and data model is per repository only
stored in the gtk.ListStore. This improves performance of many
functions,
as we are working on smaller data sets and we are not iterating
through all packages to get selections.

Switching repositories may take now more time, but this will be
improved a lot once caching will go in, but I didn't wanted to introduce
to many changes in one webrev.

This fixes bugs:

[Split the repository data models]:
http://defect.opensolaris.org/bz/show_bug.cgi?id=6516

[Package manager should elide categories which are empty]
http://defect.opensolaris.org/bz/show_bug.cgi?id=5662

[packagemanager chews CPU when changing repository]
http://defect.opensolaris.org/bz/show_bug.cgi?id=6227

[Unnecessary processing during PackageManager startup]
http://defect.opensolaris.org/bz/show_bug.cgi?id=5866

[Two entries appear in packagemanager for installed package]
From the bug description I think I was not able to reproduce, but I
didn't understand
this bug description in 100%.
http://defect.opensolaris.org/bz/show_bug.cgi?id=6226

["find as you type" performance doesn't scale]
This is just an improvement for this bug. Having per repository data
model refilter function will reduce
time to perform it, but only in some cases. Please read more in the
bug description.
http://defect.opensolaris.org/bz/show_bug.cgi?id=6365






_______________________________________________
pkg-discuss mailing list
pkg-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to