Update Portage to sync BlockerDB at init, rather than before every package.

Update Portage to sync BlockerDB at init, rather than before every package.

Currently, Portage syncs the BlockerDB before every merge. This slows down
merges considerably because it requires reading the entire vardb from disk.
Since Portage doesn't merge conflicting packages anyway, we can optimize this
by just reading the vardb at initialization and caching that. 

Change-Id: I6701926f022ef3aa2da10482fc8b09573ae24610

R=zmedico@chromium.org
BUG=none
TEST=Test merge with blockers

[diandersAtChromium, 2011-03-22 02:57:59]

I reviewed this from the perspective of a complete portage newbie, so mostly
just dumb questions.  I didn't see any obvious bugs, but I obviously don't have
a deep understanding of portage, so no promises I didn't miss anything.

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/BlockerDB.py
File pym/_emerge/BlockerDB.py (right):

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/BlockerDB.py#newcode33
pym/_emerge/BlockerDB.py:33: def findInstalledBlockers(self, new_pkg):
This is a public function.  Any issues with compatibility with other people
using portage as a python library?

...if so, those people would need to know that they can't pass acquire_lock
anymore, that they should call sync manually (at some point) and that they
should call discard_blocker.

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/BlockerDB.py#newcode43
pym/_emerge/BlockerDB.py:43: fake_vartree = self._fake_vartree
Summary: removed the sync() of self._fake_vartree

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/FakeVartree.py
File pym/_emerge/FakeVartree.py (right):

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/FakeVartree.py#newc...
pym/_emerge/FakeVartree.py:105: Discard a package from the fake vardb if it
exists.
Is pkg always a Package object?  It's never a 4-tuple, right?  It looks like
self.dbapi.get() handles the 4-tuple, and I'd like to understand if you need
that functionality here.

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/FakeVartree.py#newc...
pym/_emerge/FakeVartree.py:107: old_pkg = self.dbapi.get(pkg)
I would _love_ if it you added a docstring to PackageVirtualDbapi's get()
function so I could understand this more quickly.

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/FakeVartree.py#newc...
pym/_emerge/FakeVartree.py:108: if old_pkg:
Should check for 'is None' or use 'if pkg in self.dbapi'.

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/FakeVartree.py#newc...
pym/_emerge/FakeVartree.py:154: self.cpv_discard(pkg)
Looks like you added an extra level of indirection here.  For instance:

Old code did:
  self.dbapi.cpv_remote(pkg)

New code does:
  self.dbapi.cpv_remove(self.dbapi.get(pkg))

This looks OK since self.dbapi.get(pkg) will always return pkg in this case,
right?

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/Scheduler.py
File pym/_emerge/Scheduler.py (right):

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/Scheduler.py#newcod...
pym/_emerge/Scheduler.py:328: fake_vartree.sync()
How did you decide this was the place to put the sync()?

...this is not the only construction place for FakeVartree.  Do those other
places need sync() also?


To answer my own question:
* The only place that changed to not auto-call sync was findInstalledBlockers.
* The only caller of findInstalledBlockers is based on iterating over
self._blocker_db
* This is the only place that puts things in self._blocker_db.

...so I guess you're good?

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/Scheduler.py#newcod...
pym/_emerge/Scheduler.py:643: Returns a callable.
...that will find the blockers for the given package.

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/Scheduler.py#newcod...
pym/_emerge/Scheduler.py:652: 
Are you sure that the gc.collect() is no longer needed?  Seems like a different
CL should remove this, maybe (here and below).

[davidjames, 2011-03-23 20:19:11]

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/BlockerDB.py
File pym/_emerge/BlockerDB.py (right):

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/BlockerDB.py#newcode33
pym/_emerge/BlockerDB.py:33: def findInstalledBlockers(self, new_pkg):
On 2011/03/22 02:57:59, diandersAtChromium wrote:
> This is a public function.  Any issues with compatibility with other people
> using portage as a python library?

As we discussed in person, the _emerge package is a private namespace. So we
only need to worry about callers outside of Portage itself.

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/BlockerDB.py#newcode43
pym/_emerge/BlockerDB.py:43: fake_vartree = self._fake_vartree
On 2011/03/22 02:57:59, diandersAtChromium wrote:
> Summary: removed the sync() of self._fake_vartree

Exactly. No need to sync every time we search for blockers.

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/FakeVartree.py
File pym/_emerge/FakeVartree.py (right):

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/FakeVartree.py#newc...
pym/_emerge/FakeVartree.py:105: Discard a package from the fake vardb if it
exists.
On 2011/03/22 02:57:59, diandersAtChromium wrote:
> Is pkg always a Package object?  It's never a 4-tuple, right?  It looks like
> self.dbapi.get() handles the 4-tuple, and I'd like to understand if you need
> that functionality here.
Nope, we don't need that functionality... that said, it doesn't hurt.

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/FakeVartree.py#newc...
pym/_emerge/FakeVartree.py:107: old_pkg = self.dbapi.get(pkg)
On 2011/03/22 02:57:59, diandersAtChromium wrote:
> I would _love_ if it you added a docstring to PackageVirtualDbapi's get()
> function so I could understand this more quickly.
That's a good idea for a followup CL.

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/FakeVartree.py#newc...
pym/_emerge/FakeVartree.py:108: if old_pkg:
On 2011/03/22 02:57:59, diandersAtChromium wrote:
> Should check for 'is None' or use 'if pkg in self.dbapi'.

Done.

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/FakeVartree.py#newc...
pym/_emerge/FakeVartree.py:154: self.cpv_discard(pkg)
On 2011/03/22 02:57:59, diandersAtChromium wrote:
> Looks like you added an extra level of indirection here.  For instance:
> 
> Old code did:
>   self.dbapi.cpv_remote(pkg)
> 
> New code does:
>   self.dbapi.cpv_remove(self.dbapi.get(pkg))
> 
> This looks OK since self.dbapi.get(pkg) will always return pkg in this case,
> right?

Yup.

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/Scheduler.py
File pym/_emerge/Scheduler.py (right):

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/Scheduler.py#newcod...
pym/_emerge/Scheduler.py:328: fake_vartree.sync()
On 2011/03/22 02:57:59, diandersAtChromium wrote:
> How did you decide this was the place to put the sync()?
> 
> ...this is not the only construction place for FakeVartree.  Do those other
> places need sync() also?
> 
> 
> To answer my own question:
> * The only place that changed to not auto-call sync was findInstalledBlockers.
> * The only caller of findInstalledBlockers is based on iterating over
> self._blocker_db
> * This is the only place that puts things in self._blocker_db.
> 
> ...so I guess you're good?
Exactly right.

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/Scheduler.py#newcod...
pym/_emerge/Scheduler.py:643: Returns a callable.
On 2011/03/22 02:57:59, diandersAtChromium wrote:
> ...that will find the blockers for the given package.

Done.

http://codereview.chromium.org/6688037/diff/4/pym/_emerge/Scheduler.py#newcod...
pym/_emerge/Scheduler.py:652: 
On 2011/03/22 02:57:59, diandersAtChromium wrote:
> Are you sure that the gc.collect() is no longer needed?  Seems like a
different
> CL should remove this, maybe (here and below).

This gc.collect() call was only needed to workaround bugs in Python 2.5. Portage
has since dropped support for Python 2.5 so this is no longer applicable.

Not sure whether this needs to be split up into a separate CL.

[zmedico, 2011-03-25 01:22:28]

Thanks, I've applied this upstream:

http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=99ec2a8f...

[zmedico, 2011-03-26 16:18:23]

There's a fix here to make it discard a replaced package with the same SLOT:

http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=81cd62d4...