From 73656a34ed9f041ffbd12c6b780f43c092666181 Mon Sep 17 00:00:00 2001 From: "Bryn M. Reeves" Date: Fri, 14 Feb 2020 18:03:11 +0000 Subject: [policies] fix LinuxPolicy performance regression (test suite) Commit 285873a introduces a regression in the performance of LinuxProfile class initialisation: commit 285873a4f753822a88d475a1b030ab622bf4c72e Author: Bryan Quigley Date: Tue Feb 11 15:03:16 2020 -0800 [policies] Detect systemd use instead of hardcoding it All the patch does is to switch from testing self.init to determine whether to use the SystemdInit() class or the basic InitSystem() one, to looking for '/run/systemd/system' in the file system. This has more broad side effects than it might seem since the test suite uses LinuxPolicy() as a mock policy object for a large number of tests. Since SystemdInit() calls out to systemctl to obtain the state of the init system this both increases the run time for the tests and causes high resource use in systemd itself: PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND 1 root 20 0 177196 13504 4512 R 92.7 0.1 5:24.04 systemd This causes the run time for the Python2 and 3 test suite (plus pep8/pycodestyle) to grow from ~5s to over 1m20s on my test system: 285873a~1 real 0m5.683s 285873a real 1m20.353s Allow direct users of LinuxPolicy to override the init system detection by specifying an init= kwarg initialised to an InitSystem-like object directly, and use this in the test suite to avoid the cost of initialising the full SystemdInit() class. Resolves: #1953 Signed-off-by: Bryn M. Reeves --- tests/option_tests.py | 4 ++-- tests/plugin_tests.py | 38 +++++++++++++++++++------------------- 2 files changed, 21 insertions(+), 21 deletions(-) (limited to 'tests') diff --git a/tests/option_tests.py b/tests/option_tests.py index f0587367..4af16270 100644 --- a/tests/option_tests.py +++ b/tests/option_tests.py @@ -3,7 +3,7 @@ import unittest from sos.plugins import Plugin -from sos.policies import LinuxPolicy +from sos.policies import LinuxPolicy, InitSystem class MockOptions(object): @@ -18,7 +18,7 @@ class GlobalOptionTest(unittest.TestCase): def setUp(self): self.commons = { 'sysroot': '/', - 'policy': LinuxPolicy(), + 'policy': LinuxPolicy(init=InitSystem()), 'cmdlineopts': MockOptions() } self.plugin = Plugin(self.commons) diff --git a/tests/plugin_tests.py b/tests/plugin_tests.py index 9f03afa2..a886cfc7 100644 --- a/tests/plugin_tests.py +++ b/tests/plugin_tests.py @@ -12,7 +12,7 @@ except: from sos.plugins import Plugin, regex_findall, _mangle_command from sos.archive import TarFileArchive -from sos.policies import LinuxPolicy +from sos.policies import LinuxPolicy, InitSystem import sos.policies PATH = os.path.dirname(__file__) @@ -152,50 +152,50 @@ class PluginTests(unittest.TestCase): def setUp(self): self.mp = MockPlugin({ 'cmdlineopts': MockOptions(), - 'policy': LinuxPolicy(), + 'policy': LinuxPolicy(init=InitSystem()), 'sysroot': self.sysroot, 'cmdlineopts': MockOptions() }) self.mp.archive = MockArchive() def test_plugin_default_name(self): - p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(), + p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(init=InitSystem()), 'cmdlineopts': MockOptions()}) self.assertEquals(p.name(), "mockplugin") def test_plugin_set_name(self): - p = NamedMockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(), + p = NamedMockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(init=InitSystem()), 'cmdlineopts': MockOptions()}) self.assertEquals(p.name(), "testing") def test_plugin_no_descrip(self): - p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(), + p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(init=InitSystem()), 'cmdlineopts': MockOptions()}) self.assertEquals(p.get_description(), "") def test_plugin_no_descrip(self): - p = NamedMockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(), + p = NamedMockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(init=InitSystem()), 'cmdlineopts': MockOptions()}) self.assertEquals(p.get_description(), "This plugin has a description.") def test_set_plugin_option(self): - p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(), + p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(init=InitSystem()), 'cmdlineopts': MockOptions()}) p.set_option("opt", "testing") self.assertEquals(p.get_option("opt"), "testing") def test_set_nonexistant_plugin_option(self): - p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(), + p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(init=InitSystem()), 'cmdlineopts': MockOptions()}) self.assertFalse(p.set_option("badopt", "testing")) def test_get_nonexistant_plugin_option(self): - p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(), + p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(init=InitSystem()), 'cmdlineopts': MockOptions()}) self.assertEquals(p.get_option("badopt"), 0) def test_get_unset_plugin_option(self): - p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(), + p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(init=InitSystem()), 'cmdlineopts': MockOptions()}) self.assertEquals(p.get_option("opt"), 0) @@ -203,7 +203,7 @@ class PluginTests(unittest.TestCase): # this shows that even when we pass in a default to get, # we'll get the option's default as set in the plugin # this might not be what we really want - p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(), + p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(init=InitSystem()), 'cmdlineopts': MockOptions()}) self.assertEquals(p.get_option("opt", True), True) @@ -212,23 +212,23 @@ class PluginTests(unittest.TestCase): # if the plugin default is not None # we'll get the option's default as set in the plugin # this might not be what we really want - p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(), + p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(init=InitSystem()), 'cmdlineopts': MockOptions()}) self.assertEquals(p.get_option("opt2", True), False) def test_get_option_as_list_plugin_option(self): - p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(), + p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(init=InitSystem()), 'cmdlineopts': MockOptions()}) p.set_option("opt", "one,two,three") self.assertEquals(p.get_option_as_list("opt"), ['one', 'two', 'three']) def test_get_option_as_list_plugin_option_default(self): - p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(), + p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(init=InitSystem()), 'cmdlineopts': MockOptions()}) self.assertEquals(p.get_option_as_list("opt", default=[]), []) def test_get_option_as_list_plugin_option_not_list(self): - p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(), + p = MockPlugin({'sysroot': self.sysroot, 'policy': LinuxPolicy(init=InitSystem()), 'cmdlineopts': MockOptions()}) p.set_option("opt", "testing") self.assertEquals(p.get_option_as_list("opt"), ['testing']) @@ -245,7 +245,7 @@ class PluginTests(unittest.TestCase): p = ForbiddenMockPlugin({ 'cmdlineopts': MockOptions(), 'sysroot': self.sysroot, - 'policy': LinuxPolicy() + 'policy': LinuxPolicy(init=InitSystem()) }) p.archive = MockArchive() p.setup() @@ -256,7 +256,7 @@ class PluginTests(unittest.TestCase): p = PostprocMockPlugin({ 'cmdlineopts': MockOptions(), 'sysroot': self.sysroot, - 'policy': LinuxPolicy() + 'policy': LinuxPolicy(init=InitSystem()) }) p.postproc() self.assertTrue(p.did_postproc) @@ -269,7 +269,7 @@ class AddCopySpecTests(unittest.TestCase): def setUp(self): self.mp = MockPlugin({ 'cmdlineopts': MockOptions(), - 'policy': LinuxPolicy(), + 'policy': LinuxPolicy(init=InitSystem()), 'sysroot': os.getcwd(), 'cmdlineopts': MockOptions() }) @@ -375,7 +375,7 @@ class RegexSubTests(unittest.TestCase): def setUp(self): self.mp = MockPlugin({ 'cmdlineopts': MockOptions(), - 'policy': LinuxPolicy(), + 'policy': LinuxPolicy(init=InitSystem()), 'sysroot': os.getcwd() }) self.mp.archive = MockArchive() -- cgit