From b6cfa2905511fef1f0f179baeadde0b2a2724487 Mon Sep 17 00:00:00 2001 From: Doug Hellmann Date: Wed, 28 Mar 2018 17:18:32 -0400 Subject: [PATCH] disable stack inspection when setting option values Inspecting the stack every time we create an Opt or call set_defaults() is too expensive. This patch disables it but leaves the code in place to make it easier for someone else to come along and work on the performance issue without having to revert the patch completely. Change-Id: I25bd71ffa49f4232972370a35bf9040e7c45bf70 Addresses-Bug: #1759689 Signed-off-by: Doug Hellmann --- oslo_config/cfg.py | 33 ++++++++++++++------------ oslo_config/tests/test_get_location.py | 12 ++++++---- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/oslo_config/cfg.py b/oslo_config/cfg.py index 6ce07331..b11e8f6c 100644 --- a/oslo_config/cfg.py +++ b/oslo_config/cfg.py @@ -489,7 +489,7 @@ import copy import errno import functools import glob -import inspect +# import inspect import itertools import logging import os @@ -811,20 +811,23 @@ def _get_caller_detail(n=2): :type n: int :returns: str """ - s = inspect.stack()[:n + 1] - try: - frame = s[n] - try: - return frame[1] - # WARNING(dhellmann): Using frame.lineno to include the - # line number in the return value causes some sort of - # memory or stack corruption that manifests in values not - # being cleaned up in the cfgfilter tests. - # return '%s:%s' % (frame[1], frame[2]) - finally: - del frame - finally: - del s + return None + # FIXME(dhellmann): We need to look at the performance issues with + # doing this for every Opt instance. + # s = inspect.stack()[:n + 1] + # try: + # frame = s[n] + # try: + # return frame[1] + # # WARNING(dhellmann): Using frame.lineno to include the + # # line number in the return value causes some sort of + # # memory or stack corruption that manifests in values not + # # being cleaned up in the cfgfilter tests. + # # return '%s:%s' % (frame[1], frame[2]) + # finally: + # del frame + # finally: + # del s def set_defaults(opts, **kwargs): diff --git a/oslo_config/tests/test_get_location.py b/oslo_config/tests/test_get_location.py index b2134665..0c4e0ec5 100644 --- a/oslo_config/tests/test_get_location.py +++ b/oslo_config/tests/test_get_location.py @@ -70,7 +70,8 @@ class GetLocationTestCase(base.BaseTestCase): cfg.Locations.opt_default, loc.location, ) - self.assertIn('test_get_location.py', loc.detail) + self.assertIsNone(loc.detail) + # self.assertIn('test_get_location.py', loc.detail) def test_set_default_on_config_opt(self): self.conf.set_default('normal_opt', self.id()) @@ -80,7 +81,8 @@ class GetLocationTestCase(base.BaseTestCase): cfg.Locations.set_default, loc.location, ) - self.assertIn('test_get_location.py', loc.detail) + self.assertIsNone(loc.detail) + # self.assertIn('test_get_location.py', loc.detail) def test_set_defaults_func(self): cfg.set_defaults([self.normal_opt], normal_opt=self.id()) @@ -90,7 +92,8 @@ class GetLocationTestCase(base.BaseTestCase): cfg.Locations.set_default, loc.location, ) - self.assertIn('test_get_location.py', loc.detail) + self.assertIsNone(loc.detail) + # self.assertIn('test_get_location.py', loc.detail) def test_set_override(self): self.conf.set_override('normal_opt', self.id()) @@ -100,7 +103,8 @@ class GetLocationTestCase(base.BaseTestCase): cfg.Locations.set_override, loc.location, ) - self.assertIn('test_get_location.py', loc.detail) + self.assertIsNone(loc.detail) + # self.assertIn('test_get_location.py', loc.detail) def test_user_cli(self): filename = self._write_opt_to_tmp_file(