Nir Soffer has uploaded a new change for review.
Change subject: profiling: Decouple cpu profiling from vdsm config ......................................................................
profiling: Decouple cpu profiling from vdsm config
Previously we accessed vdsm configuration to set module defaults during import. This make it impossible to use the module from another application like supervdsm or ovirt-ha-agent.
The configuration is accessed now when starting an application wide profile using the start() and stop() interface.
This change also simplify the tests, we can mock (almost) everything via the module config.
vdsm configuration is still loaded when importing the profiler module, more work is needed to avoid this.
Change-Id: I711c7947534841f77b88c6067c992ca1a3a42fdb Signed-off-by: Nir Soffer nsoffer@redhat.com --- M lib/vdsm/profiling/cpu.py M tests/cpu_profile_test.py 2 files changed, 37 insertions(+), 56 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/53/65253/1
diff --git a/lib/vdsm/profiling/cpu.py b/lib/vdsm/profiling/cpu.py index 286fcdd..427f1db 100644 --- a/lib/vdsm/profiling/cpu.py +++ b/lib/vdsm/profiling/cpu.py @@ -39,10 +39,6 @@ # Defaults
_FILENAME = os.path.join(constants.P_VDSM_RUN, 'vdsmd.prof') -_FORMAT = config.get('devel', 'cpu_profile_format') -_BUILTINS = config.getboolean('devel', 'cpu_profile_builtins') -_CLOCK = config.get('devel', 'cpu_profile_clock') -_THREADS = True
_lock = threading.Lock() _profiler = None @@ -105,8 +101,12 @@ with _lock: if _profiler: raise UsageError('CPU profiler is already running') - _profiler = Profiler(_FILENAME, format=_FORMAT, clock=_CLOCK, - builtins=_BUILTINS, threads=_THREADS) + _profiler = Profiler( + _FILENAME, + format=config.get('devel', 'cpu_profile_format'), + clock=config.get('devel', 'cpu_profile_clock'), + builtins=config.getboolean('devel', 'cpu_profile_builtins'), + threads=True) _profiler.start()
@@ -128,8 +128,8 @@ return yappi and yappi.is_running()
-def profile(filename, format=_FORMAT, clock=_CLOCK, builtins=_BUILTINS, - threads=_THREADS): +def profile(filename, format='pstat', clock='cpu', builtins=True, + threads=True): """ Profile decorated function, saving profile to filename using format.
diff --git a/tests/cpu_profile_test.py b/tests/cpu_profile_test.py index 882b7b6..850d5e8 100644 --- a/tests/cpu_profile_test.py +++ b/tests/cpu_profile_test.py @@ -55,12 +55,19 @@ raise
+def config(enable='true', format='pstat', clock='cpu', builtins='false'): + return make_config([ + ('devel', 'cpu_profile_enable', enable), + ('devel', 'cpu_profile_format', format), + ('devel', 'cpu_profile_clock', clock), + ('devel', 'cpu_profile_builtins', builtins), + ]) + + class ApplicationProfileTests(ProfileTests):
- @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'true')])) + @MonkeyPatch(cpu, 'config', config()) @MonkeyPatch(cpu, '_FILENAME', FILENAME) - @MonkeyPatch(cpu, '_FORMAT', 'pstat') def test_pstats_format(self): requires_yappi() cpu.start() @@ -68,10 +75,8 @@ cpu.stop() self.assertNotRaises(pstats.Stats, FILENAME)
- @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'true')])) + @MonkeyPatch(cpu, 'config', config(format='ystat')) @MonkeyPatch(cpu, '_FILENAME', FILENAME) - @MonkeyPatch(cpu, '_FORMAT', 'ystat') def test_ystats_format(self): requires_yappi() cpu.start() @@ -79,11 +84,8 @@ cpu.stop() self.assertNotRaises(open_ystats, FILENAME)
- @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'true')])) + @MonkeyPatch(cpu, 'config', config(format='ystat', builtins='true')) @MonkeyPatch(cpu, '_FILENAME', FILENAME) - @MonkeyPatch(cpu, '_FORMAT', 'ystat') - @MonkeyPatch(cpu, '_BUILTINS', True) def test_with_builtins(self): requires_yappi() cpu.start() @@ -92,11 +94,8 @@ stats = open_ystats(FILENAME) self.assertTrue(find_module(stats, '__builtin__'))
- @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'true')])) + @MonkeyPatch(cpu, 'config', config(format='ystat', builtins='false')) @MonkeyPatch(cpu, '_FILENAME', FILENAME) - @MonkeyPatch(cpu, '_FORMAT', 'ystat') - @MonkeyPatch(cpu, '_BUILTINS', False) def test_without_builtins(self): requires_yappi() cpu.start() @@ -106,10 +105,8 @@ self.assertFalse(find_module(stats, '__builtin__'))
@MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'true')])) + config(format='ystat', clock='cpu', builtins='false')) @MonkeyPatch(cpu, '_FILENAME', FILENAME) - @MonkeyPatch(cpu, '_FORMAT', 'ystat') - @MonkeyPatch(cpu, '_CLOCK', 'cpu') def test_cpu_clock(self): requires_yappi() cpu.start() @@ -121,10 +118,8 @@ self.assertTrue(func.ttot < 0.1)
@MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'true')])) + config(format='ystat', clock='wall', builtins='false')) @MonkeyPatch(cpu, '_FILENAME', FILENAME) - @MonkeyPatch(cpu, '_FORMAT', 'ystat') - @MonkeyPatch(cpu, '_CLOCK', 'wall') def test_wall_clock(self): requires_yappi() cpu.start() @@ -135,8 +130,7 @@ func = find_function(stats, __file__, name) self.assertTrue(func.ttot > 0.1)
- @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'true')])) + @MonkeyPatch(cpu, 'config', config()) @MonkeyPatch(cpu, '_FILENAME', FILENAME) def test_is_running(self): requires_yappi() @@ -148,15 +142,13 @@ cpu.stop() self.assertFalse(cpu.is_running())
- @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'true')])) + @MonkeyPatch(cpu, 'config', config(enable='true')) def test_is_enabled(self): requires_yappi() self.assertTrue(cpu.is_enabled())
# This must succeed even if yappi is not installed - @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'false')])) + @MonkeyPatch(cpu, 'config', config(enable='false')) def test_disabled(self): cpu.start() try: @@ -171,8 +163,7 @@ class FunctionProfileTests(ProfileTests):
# Function profile must succeed if profile is disabled in config. - @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'false')])) + @MonkeyPatch(cpu, 'config', config(enable='false')) def test_profile_disabled(self): requires_yappi() self.profiled_function() @@ -180,8 +171,7 @@
# Function profile must fail if profile is enabled in config - we cannot # use application wide profile and function profile in the same time. - @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'true')])) + @MonkeyPatch(cpu, 'config', config(enable='true')) @MonkeyPatch(cpu, '_FILENAME', FILENAME) def test_fail_if_Profile_is_running(self): requires_yappi() @@ -193,38 +183,33 @@ cpu.stop()
# It is not possible to call a profiled function from a profiled function. - @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'false')])) + @MonkeyPatch(cpu, 'config', config(enable='false')) def test_fail_recursive_profile(self): requires_yappi() self.assertRaises(UsageError, self.recursive_profile)
- @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'false')])) + @MonkeyPatch(cpu, 'config', config(enable='false')) def test_ystat_format(self): requires_yappi() self.ystat_format() self.assertNotRaises(open_ystats, FILENAME)
- @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'false')])) + @MonkeyPatch(cpu, 'config', config(enable='false')) def test_with_builtins(self): requires_yappi() self.with_builtins() stats = open_ystats(FILENAME) self.assertTrue(find_module(stats, '__builtin__'))
- @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'false')])) + @MonkeyPatch(cpu, 'config', config(enable='false')) def test_without_builtins(self): requires_yappi() self.without_builtins() stats = open_ystats(FILENAME) self.assertFalse(find_module(stats, '__builtin__'))
- @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'false')])) + @MonkeyPatch(cpu, 'config', config(enable='false')) def test_cpu_clock(self): requires_yappi() self.cpu_clock() @@ -233,8 +218,7 @@ func = find_function(stats, __file__, name) self.assertTrue(func.ttot < 0.1)
- @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'false')])) + @MonkeyPatch(cpu, 'config', config(enable='false')) def test_wall_clock(self): requires_yappi() self.wall_clock() @@ -279,8 +263,7 @@ self.ready = threading.Event() self.resume = threading.Event()
- @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'false')])) + @MonkeyPatch(cpu, 'config', config(enable='false')) def test_new_threads(self): # The easy case - threads started after yappi was started requires_yappi() @@ -290,8 +273,7 @@ func = find_function(stats, __file__, name) self.assertEquals(func.ncall, 1)
- @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'false')])) + @MonkeyPatch(cpu, 'config', config(enable='false')) def test_running_threads(self): # The harder case - threads started before yappi was started requires_yappi() @@ -302,8 +284,7 @@ func = find_function(stats, __file__, name) self.assertEquals(func.ncall, 1)
- @MonkeyPatch(cpu, 'config', - make_config([('devel', 'cpu_profile_enable', 'false')])) + @MonkeyPatch(cpu, 'config', config(enable='false')) def test_without_threads(self): requires_yappi() self.without_threads()
gerrit-hooks has posted comments on this change.
Change subject: profiling: Decouple cpu profiling from vdsm config ......................................................................
Patch Set 1:
* Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Nir Soffer has posted comments on this change.
Change subject: profiling: Decouple cpu profiling from vdsm config ......................................................................
Patch Set 1: Verified+1
vdsm-patches@lists.fedorahosted.org