Merge lp:~kissiel/checkbox/speed-optimization into lp:checkbox

Proposed by Maciej Kisielewski on 2016-10-04
Status: Merged
Approved by: Maciej Kisielewski on 2016-10-05
Approved revision: 4520
Merged at revision: 4517
Proposed branch: lp:~kissiel/checkbox/speed-optimization
Merge into: lp:checkbox
Diff against target: 71 lines (+6/-8)
4 files modified
checkbox-ng/checkbox_ng/misc.py (+1/-1)
plainbox/plainbox/impl/session/state.py (+1/-1)
plainbox/plainbox/impl/unit/test_unit.py (+0/-3)
plainbox/plainbox/impl/unit/unit.py (+4/-3)
To merge this branch: bzr merge lp:~kissiel/checkbox/speed-optimization
Reviewer Review Type Date Requested Status
Po-Hsu Lin Approve on 2016-10-05
Paul Larson 2016-10-04 Approve on 2016-10-04
Review via email: mp+307553@code.launchpad.net

Description of the change

This MR is the first (and simple) patch set that improves plainbox startup time.

The speed-up depends on how many jobs there are in your provider 'database'.

Here are benchmarks from my slow NUC:

1000 jobs - 44s -> 24s
2500 jobs - 2m43s -> 1m19s

The benchmarks were done using this tool:
https://code.launchpad.net/~kissiel/+git/benchcbox/+ref/master

The tool generates providers and checkbox-cli launcher that doesn't run any of the jobs. Measuring wall time of running this launcher gives an estimate how long checkbox takes to 'spool up'.

To post a comment you must log in.
Paul Larson (pwlars) wrote :

Wow, part of me really wants to pick this apart and find if there are even more gains, compare it to normal sets, and things like that, but I see no reason to block such an obvious improvement for any time at all. It's hard to argue with those results, this is great!

review: Approve
Po-Hsu Lin (cypressyew) wrote :

Cool stuff, thanks.

review: Approve
Sylvain Pineau (sylvain-pineau) wrote :
Download full text (10.0 KiB)

The attempt to merge lp:~kissiel/checkbox/speed-optimization into lp:checkbox failed. Below is the output from the failed tests.

[precise] starting container
[precise] (timing) 0.11user 0.03system 0:05.50elapsed 2%CPU (0avgtext+0avgdata 9960maxresident)k
[precise] (timing) 0inputs+32outputs (0major+6827minor)pagefaults 0swaps
[precise] provisioning container
[precise] (timing) 45.51user 18.60system 1:07.10elapsed 95%CPU (0avgtext+0avgdata 97156maxresident)k
[precise] (timing) 0inputs+19168outputs (0major+3199615minor)pagefaults 0swaps
[precise-testing] Starting tests...
Found a test script: ./checkbox-ng/requirements/container-tests-checkbox-ng-unit
[precise-testing] container-tests-checkbox-ng-unit: FAIL
[precise-testing] stdout: http://paste.ubuntu.com/23278019/
[precise-testing] stderr: http://paste.ubuntu.com/23278020/
[precise-testing] (timing) Command exited with non-zero status 1
[precise-testing] (timing) 0.69user 0.14system 0:00.85elapsed 98%CPU (0avgtext+0avgdata 48608maxresident)k
[precise-testing] (timing) 0inputs+1280outputs (0major+22164minor)pagefaults 0swaps
Found a test script: ./checkbox-support/requirements/container-tests-checkbox-support
[precise-testing] container-tests-checkbox-support: PASS
[precise-testing] (timing) 31.22user 0.35system 0:31.61elapsed 99%CPU (0avgtext+0avgdata 150684maxresident)k
[precise-testing] (timing) 0inputs+1376outputs (0major+39712minor)pagefaults 0swaps
Found a test script: ./converged/requirements/container-tests-touch-unit-tests
[precise-testing] container-tests-touch-unit-tests: PASS
[precise-testing] (timing) 0.00user 0.02system 0:00.02elapsed 86%CPU (0avgtext+0avgdata 2232maxresident)k
[precise-testing] (timing) 0inputs+8outputs (0major+2345minor)pagefaults 0swaps
Found a test script: ./plainbox/plainbox/impl/providers/categories/requirements/container-tests-provider-categories
[precise-testing] container-tests-provider-categories: PASS
[precise-testing] (timing) 1.05user 0.09system 0:01.15elapsed 99%CPU (0avgtext+0avgdata 47200maxresident)k
[precise-testing] (timing) 0inputs+64outputs (0major+14466minor)pagefaults 0swaps
Found a test script: ./plainbox/requirements/001-container-tests-plainbox-egg-info
[precise-testing] 001-container-tests-plainbox-egg-info: PASS
[precise-testing] (timing) 0.22user 0.05system 0:00.28elapsed 98%CPU (0avgtext+0avgdata 14568maxresident)k
[precise-testing] (timing) 0inputs+88outputs (0major+6046minor)pagefaults 0swaps
Found a test script: ./plainbox/requirements/container-tests-plainbox
[precise-testing] container-tests-plainbox: PASS
[precise-testing] (timing) 38.72user 1.39system 0:40.26elapsed 99%CPU (0avgtext+0avgdata 201576maxresident)k
[precise-testing] (timing) 0inputs+3304outputs (0major+263264minor)pagefaults 0swaps
Found a test script: ./plainbox/requirements/container-tests-plainbox-documentation
[precise-testing] container-tests-plainbox-documentation: PASS
[precise-testing] (timing) 102.37user 0.59system 1:43.22elapsed 99%CPU (0avgtext+0avgdata 185420maxresident)k
[precise-testing] (timing) 0inputs+43216outputs (0major+58278minor)pagefaults 0swaps...

4520. By Maciej Kisielewski on 2016-10-05

checkbox-ng:misc: use expanded tuple for comparison

This way we check for 'not None' instead of going through full __eq__ chain.

Signed-off-by: Maciej Kisielewski <email address hidden>

Maciej Kisielewski (kissiel) wrote :

r.4520 fixes the failing test by unpacking (None, None) tuple and checking 'is not None' for both parts of the pair individually.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'checkbox-ng/checkbox_ng/misc.py'
2--- checkbox-ng/checkbox_ng/misc.py 2016-03-11 13:46:41 +0000
3+++ checkbox-ng/checkbox_ng/misc.py 2016-10-05 09:15:30 +0000
4@@ -298,7 +298,7 @@
5 else:
6 tree.current_index += 1
7 result = category.get_node_by_index(index, tree)
8- if result != (None, None):
9+ if result[0] is not None and result[1] is not None:
10 return result
11 for job in self.jobs:
12 if index == tree.current_index:
13
14=== modified file 'plainbox/plainbox/impl/session/state.py'
15--- plainbox/plainbox/impl/session/state.py 2016-06-09 07:28:53 +0000
16+++ plainbox/plainbox/impl/session/state.py 2016-10-05 09:15:30 +0000
17@@ -413,7 +413,7 @@
18
19 This method fires the :meth:`on_unit_added()` signal
20 """
21- if unit in self._unit_list:
22+ if unit in frozenset(self._unit_list):
23 raise ValueError(
24 _("attempting to add the same unit twice: %s" % unit.id))
25 self.state.add_unit(unit)
26
27=== modified file 'plainbox/plainbox/impl/unit/test_unit.py'
28--- plainbox/plainbox/impl/unit/test_unit.py 2016-05-20 13:54:59 +0000
29+++ plainbox/plainbox/impl/unit/test_unit.py 2016-10-05 09:15:30 +0000
30@@ -262,9 +262,6 @@
31 self.assertNotEqual(
32 Unit({}, parameters={'param': 'value1'}),
33 Unit({}, parameters={'param': 'value2'}))
34- # Ensure that units are not equal to other classes
35- self.assertTrue(Unit({}) != object())
36- self.assertFalse(Unit({}) == object())
37
38 def test_get_accessed_parameters(self):
39 # There are no accessed parameters if the unit is not parameterized
40
41=== modified file 'plainbox/plainbox/impl/unit/unit.py'
42--- plainbox/plainbox/impl/unit/unit.py 2016-09-28 20:10:43 +0000
43+++ plainbox/plainbox/impl/unit/unit.py 2016-10-05 09:15:30 +0000
44@@ -383,6 +383,7 @@
45 self._checksum = None
46 self._parameters = parameters
47 self._virtual = virtual
48+ self._hash_cache = None
49
50 @classmethod
51 def instantiate_template(cls, data, raw_data, origin, provider, parameters,
52@@ -405,8 +406,6 @@
53 field_offset_map)
54
55 def __eq__(self, other):
56- if not isinstance(other, Unit):
57- return False
58 return self.checksum == other.checksum
59
60 def __ne__(self, other):
61@@ -415,7 +414,9 @@
62 return self.checksum != other.checksum
63
64 def __hash__(self):
65- return hash(self.checksum)
66+ if self._hash_cache is None:
67+ self._hash_cache = int(self.checksum, 16)
68+ return self._hash_cache
69
70 @property
71 def unit(self):

Subscribers

People subscribed via source and target branches