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?
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.
Great, this sounds like the right functionality then. Just wanted to
make sure.
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.
I'm glad John's going to be doing that. Given that, I'm not sure why the
refactoring of the data (in this way) is needed.
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.
Ok, stopping the multiple refiltering seems good :)
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.
I don't think I agree that 6516 is needed for caching. That's to say,
you might want to cache by repository, but not organize your in memory
store of packages along those lines. Actually, I don't really care how
the data's arranged in memory, but I care about the UI implications that
seems to have. If you have a plan for presenting a unified view of
packages across repos, given this split data model, that's great. It
seems counterintuitive to me though, which is why I brought up this
issue in the first place. I would generally expect the data backing the
UI to share it's basic structure with the UI. Because I'm advocating for
a different UI experience, I think that moving to a data store further
from that UI structure is potentially problematic.
Brock
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
