Britney2/ITM: Performance, improve break-arch handling and minor cleanup/refactoring
Hi,
I intend to merge the following 6 patches into the Britney master
branch no later than Thursday the 14th of August (provided no one
objects) and I kindly ask you to review them before then.
The two major benefits from these patches are:
* 30+ minute runtime reduction on the Kali dataset.
- Primarily from patch 0006
* do_all now automatically detects hints that only apply to
"break-arch" migrations and temporarily disables the "break arches"
to avoid nuninst regressions.
- This applies to manual *and* automatic hints. The only requirement
is that only "break arch" migrations are involved. It does *not*
change the handling of "per break arch" main runs.
- Provided by the patches 0003 and 0004
A short note on each of the patches (in order):
* 0001-britney.py-Save-a-table-look-up.patch
- 1-line clean up to avoid repeated hashtable look up
* 0002-britney.py-Avoid-making-migration-items-from-migrati.patch
- Minor clean up in iter_packages to avoid making migration items
out of migration items.
* 0003-Move-the-nuninst-checker-function-to-britney_util.patch
- Refactor is_nuninst_asgood_generous and move it to britney-util
* 0004-Avoid-unnecessary-nuninst-regressions-for-break-arch.patch
- Have do_all detect when all migrations only affected "break-arches"
and use it to avoid nuninst regressions.
- Depends on patch 0003
* 0005-britney.py-Skip-lundo-maintenance-in-non-hint-recurs.patch
- During a main run, iter_packages have maintained the "lundo" list,
which serves two purposes.
1. It allows us to undo all items that iter_packages migrated
2. It is allegedly needed for some cases for "hint"-hints
(traced to #625792)
During the "main run", Britney immediately accepts or rejects
packages and will *never* back out of them once accepted.
Therefore, neither use cases for "lundo" apply to "recurse" runs
outside "hint"-hints.
NB: Previously lundo was also used for undoing failed "easy" hints,
but since August 4th "easy"-hints no longer invoke iter_packages.
They now trigger calls to "iter_packages_hint" instead.
* 0006-do_all-Only-sort_actions-after-recurse-runs.patch
- This patch avoids calling to "sort_actions" after successful
"easy"-hints. This reduces the runtime of the Kali dataset by
30-35 minutes, where each call to sort_actions costs no less than
3 full minutes.
- Note that easy hints are not affected by the order of actions in
"upgrade_me", so calls to sort_actions before an "easy"-hint are
meaningless. Furthermore, a successful "easy"-hint will perserve
the current order of "upgrade_me", so sort_actions after an
"easy"-hint is generally also fairly useless[1].
Test results:
* All test cases passes (with no changes to "expected" results)
* The resulting HeidiResult file of live-2012-05-09 do change due to
patch 0003 and 0004 (caused by some auto hints for "hurd-i386" that
now fails).
- It is the only test case with break arches (added to have a
"bootstrap a new arch in testing" test case).
- However, there are no "desired" result for this test case, so
this test passes as long as Britney does not crash.
~Niels
[1] There might be theoretical cases, where a call to sort_actions after
a successful "easy"-hint could slightly improve the ordering. However,
I suspect that such cases needs hinting to migrate and that sort_actions
will not help the recurse run...
>From f88bcd98a2e82406b747388fe0e7b81ae4f9d4ac Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Mon, 28 Jul 2014 22:13:30 +0200
Subject: [PATCH 1/6] britney.py: Save a table look up
Signed-off-by: Niels Thykier <niels@thykier.net>
---
britney.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/britney.py b/britney.py
index 654c041..9372a7a 100755
--- a/britney.py
+++ b/britney.py
@@ -1093,7 +1093,7 @@ class Britney(object):
binary_u = self.binaries[suite][arch][0][pkg_name]
# this is the source version for the new binary package
- pkgsv = self.binaries[suite][arch][0][pkg_name][SOURCEVER]
+ pkgsv = binary_u[SOURCEVER]
# if the new binary package is architecture-independent, then skip it
if binary_u[ARCHITECTURE] == 'all':
--
2.0.1
>From d93f299fe56d5e8c12910da7018854e1a615ed98 Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Tue, 29 Jul 2014 21:44:51 +0200
Subject: [PATCH 2/6] britney.py: Avoid making migration items from migration
items
Signed-off-by: Niels Thykier <niels@thykier.net>
---
britney.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/britney.py b/britney.py
index 9372a7a..a55d950 100755
--- a/britney.py
+++ b/britney.py
@@ -2234,8 +2234,8 @@ class Britney(object):
defer = False
for p in dependencies.get(item, []):
if p in skipped:
- deferred.append(make_migrationitem(item, self.sources))
- skipped.append(make_migrationitem(item, self.sources))
+ deferred.append(item)
+ skipped.append(item)
defer = True
break
if defer: continue
--
2.0.1
>From a2d63a1c730f41350c1737ad3852b39bd291e44e Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Sat, 2 Aug 2014 22:15:54 +0200
Subject: [PATCH 3/6] Move the nuninst checker function to britney_util
Signed-off-by: Niels Thykier <niels@thykier.net>
---
britney.py | 20 ++++++++++----------
britney_util.py | 21 +++++++++++++++++++++
2 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/britney.py b/britney.py
index a55d950..2f9cb50 100755
--- a/britney.py
+++ b/britney.py
@@ -218,7 +218,7 @@ from britney_util import (old_libraries_format, same_source, undo_changes,
read_nuninst, write_nuninst, write_heidi,
eval_uninst, newly_uninst, make_migrationitem,
write_excuses, write_heidi_delta, write_controlfiles,
- old_libraries)
+ old_libraries, is_nuninst_asgood_generous)
from consts import (VERSION, SECTION, BINARIES, MAINTAINER, FAKESRC,
SOURCE, SOURCEVER, ARCHITECTURE, DEPENDS, CONFLICTS,
PROVIDES, RDEPENDS, RCONFLICTS, MULTIARCH, ESSENTIAL)
@@ -1751,14 +1751,6 @@ class Britney(object):
return "%d+%d: %s" % (total, totalbreak, ":".join(res))
- def is_nuninst_asgood_generous(self, old, new):
- diff = 0
- for arch in self.options.architectures:
- if arch in self.options.break_arches.split(): continue
- diff = diff + (len(new[arch]) - len(old[arch]))
- return diff <= 0
-
-
def _compute_groups(self, source_name, suite, migration_architecture,
is_removal, include_hijacked=False,
allow_smooth_updates=True,
@@ -2325,6 +2317,7 @@ class Britney(object):
recurse = True
lundo = None
nuninst_end = None
+ better = True
extra = () # empty tuple
if hinttype == "easy" or hinttype == "force-hint":
@@ -2372,7 +2365,14 @@ class Britney(object):
self.output_write(eval_uninst(self.options.architectures,
newly_uninst(nuninst_start, nuninst_end)) + "\n")
- if force or self.is_nuninst_asgood_generous(self.nuninst_orig, nuninst_end):
+ if not force:
+ break_arches = self.options.break_arches.split()
+ better = is_nuninst_asgood_generous(self.options.architectures,
+ self.nuninst_orig,
+ nuninst_end,
+ break_arches)
+
+ if better:
# Result accepted either by force or by being better than the original result.
if recurse:
self.output_write("Apparently successful\n")
diff --git a/britney_util.py b/britney_util.py
index a931dfd..2e14870 100644
--- a/britney_util.py
+++ b/britney_util.py
@@ -574,3 +574,24 @@ def old_libraries(sources, packages, same_source=same_source):
migration = "-" + "/".join((pkg_name, arch, pkg[SOURCEVER]))
removals.append(MigrationItem(migration))
return removals
+
+
+def is_nuninst_asgood_generous(architectures, old, new, break_arches=frozenset()):
+ """Compares the nuninst counters to see if they improved
+
+ Given a list of architecters, the previous and the current nuninst
+ counters, this function determines if the current nuninst counter
+ is better than the previous one. Optionally it also accepts a set
+ of "break_arches", the nuninst counter for any architecture listed
+ in this set are completely ignored.
+
+ Returns True if the new nuninst counter is better than the
+ previous. Returns False otherwise.
+
+ """
+ diff = 0
+ for arch in architectures:
+ if arch in break_arches:
+ continue
+ diff = diff + (len(new[arch]) - len(old[arch]))
+ return diff <= 0
--
2.0.1
>From 04afa83ad88a64499363a6ae96465f043fa769f8 Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Sat, 2 Aug 2014 22:31:15 +0200
Subject: [PATCH 4/6] Avoid unnecessary nuninst regressions for break archs
The "do_all"-method now checks the architectures of all changes
applied. If they entirely consist of items from "break archs", then
"do_all" will disregard the current "break archs" setting when
comparing nuninst counters.
This change avoids unintended installability regressions on break
arches when a hint (manual or automatic) apply only to packages on
break arches.
Signed-off-by: Niels Thykier <niels@thykier.net>
---
britney.py | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/britney.py b/britney.py
index 2f9cb50..986213a 100755
--- a/britney.py
+++ b/britney.py
@@ -2366,7 +2366,12 @@ class Britney(object):
newly_uninst(nuninst_start, nuninst_end)) + "\n")
if not force:
- break_arches = self.options.break_arches.split()
+ break_arches = set(self.options.break_arches.split())
+ if all(x.architecture in break_arches for x in selected):
+ # If we only migrated items from break-arches, then we
+ # do not allow any regressions on these architectures.
+ # This usually only happens with hints
+ break_arches = set()
better = is_nuninst_asgood_generous(self.options.architectures,
self.nuninst_orig,
nuninst_end,
--
2.0.1
>From 822d847c119f6b0f87b4cdc73a65df4de1a55b67 Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Mon, 4 Aug 2014 22:32:15 +0200
Subject: [PATCH 5/6] britney.py: Skip lundo maintenance in non-hint recurse
runs
There are no uses of "lundo" left for a non-hint recurse run (i.e.
the "main run"), so there is no point in building it.
The "lundo"-list is still used in the recurse run of a "hint"-hint.
Signed-off-by: Niels Thykier <niels@thykier.net>
---
britney.py | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/britney.py b/britney.py
index 986213a..e8e46f4 100755
--- a/britney.py
+++ b/britney.py
@@ -1915,7 +1915,7 @@ class Britney(object):
return (adds, rms, set(smoothbins.itervalues()))
- def doop_source(self, item, hint_undo=[], removals=frozenset()):
+ def doop_source(self, item, hint_undo=None, removals=frozenset()):
"""Apply a change to the testing distribution as requested by `pkg`
An optional list of undo actions related to packages processed earlier
@@ -2048,7 +2048,7 @@ class Britney(object):
affected.update(get_reverse_tree(j, parch))
old_version = old_pkg_data[VERSION]
inst_tester.remove_testing_binary(binary, old_version, parch)
- else:
+ elif hint_undo:
# the binary isn't in testing, but it may have been at
# the start of the current hint and have been removed
# by an earlier migration. if that's the case then we
@@ -2205,9 +2205,6 @@ class Britney(object):
dependencies = self.dependencies
check_packages = partial(self._check_packages, binaries)
- if lundo is None:
- lundo = []
-
self.output_write("recur: [%s] %s %d/%d\n" % ("", ",".join(x.uvname for x in selected), len(packages), len(extra)))
# loop on the packages (or better, actions)
@@ -2261,7 +2258,8 @@ class Britney(object):
# check if the action improved the uninstallability counters
if better:
- lundo.append((undo, item))
+ if lundo is not None:
+ lundo.append((undo, item))
selected.append(item)
packages.extend(extra)
extra = []
--
2.0.1
>From bae9338a74b220fce6a38909d05d8359b722d30e Mon Sep 17 00:00:00 2001
From: Niels Thykier <niels@thykier.net>
Date: Tue, 5 Aug 2014 22:44:14 +0200
Subject: [PATCH 6/6] do_all(): Only sort_actions after recurse runs
sort_actions() can be quite expensive and it is wasteful to resort
actions after each successful "easy"-hint.
Signed-off-by: Niels Thykier <niels@thykier.net>
---
britney.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/britney.py b/britney.py
index e8e46f4..da0f158 100755
--- a/britney.py
+++ b/britney.py
@@ -2316,7 +2316,7 @@ class Britney(object):
lundo = None
nuninst_end = None
better = True
- extra = () # empty tuple
+ extra = []
if hinttype == "easy" or hinttype == "force-hint":
force = hinttype == "force-hint"
@@ -2395,10 +2395,10 @@ class Britney(object):
self.all_selected += selected
if not actions:
if recurse:
- self.upgrade_me = sorted(extra)
+ self.upgrade_me = extra
+ self.sort_actions()
else:
self.upgrade_me = [x for x in self.upgrade_me if x not in set(selected)]
- self.sort_actions()
else:
self.output_write("FAILED\n")
if not lundo: return
--
2.0.1
Reply to: