{% endif %} {{ panel.nav_title }} {% if panel.enabled %} From f92aba225936fa768c098a8f1db51125ac1715ec Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sun, 10 Nov 2013 10:10:28 +0100 Subject: [PATCH 2/4] Disable instrumentation for disabled panels. This eliminates the performance overhead when a panel is disabled. --- debug_toolbar/middleware.py | 10 ++++++---- debug_toolbar/panels/__init__.py | 27 ++++++++++++++++++++------- debug_toolbar/panels/cache.py | 25 +++++++++++++++---------- debug_toolbar/panels/logger.py | 3 +++ debug_toolbar/panels/sql.py | 25 ++++++++++--------------- debug_toolbar/panels/template.py | 21 +++++++++++++-------- tests/panels/test_sql.py | 5 +++++ tests/panels/test_template.py | 9 +++++++-- 8 files changed, 79 insertions(+), 46 deletions(-) diff --git a/debug_toolbar/middleware.py b/debug_toolbar/middleware.py index 4f5b9bd91..80e930508 100644 --- a/debug_toolbar/middleware.py +++ b/debug_toolbar/middleware.py @@ -95,6 +95,7 @@ def process_request(self, request): panel.enabled = panel.dom_id() not in request.COOKIES if not panel.enabled: continue + panel.enable_instrumentation() panel.process_request(request) self.__class__.debug_toolbars[threading.current_thread().ident] = toolbar @@ -130,12 +131,13 @@ def process_response(self, request, response): {'redirect_to': redirect_to} ) response.cookies = cookies + for panel in toolbar.panels: + if not panel.enabled: + continue + panel.process_response(request, response) + panel.disable_instrumentation() if ('gzip' not in response.get('Content-Encoding', '') and response.get('Content-Type', '').split(';')[0] in _HTML_TYPES): - for panel in toolbar.panels: - if not panel.enabled: - continue - panel.process_response(request, response) response.content = replace_insensitive( force_text(response.content, encoding=settings.DEFAULT_CHARSET), self.tag, diff --git a/debug_toolbar/panels/__init__.py b/debug_toolbar/panels/__init__.py index d4a61dae1..9586e80ec 100644 --- a/debug_toolbar/panels/__init__.py +++ b/debug_toolbar/panels/__init__.py @@ -22,31 +22,44 @@ class DebugPanel(object): context = {} # Panel methods + def __init__(self, toolbar, context={}): self.toolbar = toolbar self.context.update(context) self.slug = slugify(self.name) + def content(self): + if self.has_content: + context = self.context.copy() + context.update(self.get_stats()) + return render_to_string(self.template, context) + def dom_id(self): return 'djDebug%sPanel' % (self.name.replace(' ', '')) + # Titles and subtitles + def nav_title(self): - """Title showing in toolbar""" + """Title showing in sidebar""" raise NotImplementedError def nav_subtitle(self): - """Subtitle showing until title in toolbar""" + """Subtitle showing under title in sidebar""" return '' def title(self): """Title showing in panel""" raise NotImplementedError - def content(self): - if self.has_content: - context = self.context.copy() - context.update(self.get_stats()) - return render_to_string(self.template, context) + # Enable and disable (expensive) instrumentation + + def enable_instrumentation(self): + pass + + def disable_instrumentation(self): + pass + + # Store and retrieve stats (shared between panels) def record_stats(self, stats): panel_stats = self.toolbar.stats.get(self.slug) diff --git a/debug_toolbar/panels/cache.py b/debug_toolbar/panels/cache.py index 57da54b71..a6b06ce9f 100644 --- a/debug_toolbar/panels/cache.py +++ b/debug_toolbar/panels/cache.py @@ -6,7 +6,7 @@ from django.conf import settings from django.core import cache -from django.core.cache import get_cache as base_get_cache +from django.core.cache import cache as original_cache, get_cache as original_get_cache from django.core.cache.backends.base import BaseCache from django.dispatch import Signal from django.template import Node @@ -123,6 +123,10 @@ def decr_version(self, *args, **kwargs): return self.cache.decr_version(*args, **kwargs) +def get_cache(*args, **kwargs): + return CacheStatTracker(original_get_cache(*args, **kwargs)) + + class CacheDebugPanel(DebugPanel): """ Panel that displays the cache statistics. @@ -195,6 +199,16 @@ def title(self): 'Cache calls from %(count)d backends', count) % dict(count=count) + def enable_instrumentation(self): + # This isn't thread-safe because cache connections aren't thread-local + # in Django, unlike database connections. + cache.cache = CacheStatTracker(cache.cache) + cache.get_cache = get_cache + + def disable_instrumentation(self): + cache.cache = original_cache + cache.get_cache = original_get_cache + def process_response(self, request, response): self.record_stats({ 'total_calls': len(self.calls), @@ -204,12 +218,3 @@ def process_response(self, request, response): 'misses': self.misses, 'counts': self.counts, }) - - -def get_cache_debug(*args, **kwargs): - base_cache = base_get_cache(*args, **kwargs) - return CacheStatTracker(base_cache) - - -cache.cache = CacheStatTracker(cache.cache) -cache.get_cache = get_cache_debug diff --git a/debug_toolbar/panels/logger.py b/debug_toolbar/panels/logger.py index 644de30bd..45c995aab 100644 --- a/debug_toolbar/panels/logger.py +++ b/debug_toolbar/panels/logger.py @@ -73,6 +73,9 @@ def emit(self, record): logging.root.setLevel(logging.NOTSET) logging.root.addHandler(logging_handler) # register with logging +# We don't use enable/disable_instrumentation because we can't make these +# functions thread-safe and (hopefully) logging isn't too expensive. + try: import logbook logbook_supported = True diff --git a/debug_toolbar/panels/sql.py b/debug_toolbar/panels/sql.py index 1ced04915..53b9f5327 100644 --- a/debug_toolbar/panels/sql.py +++ b/debug_toolbar/panels/sql.py @@ -4,28 +4,13 @@ from copy import copy from django.db import connections -from django.db.backends import BaseDatabaseWrapper from django.utils.translation import ugettext_lazy as _, ungettext_lazy as __ from debug_toolbar.forms import SQLSelectForm -from debug_toolbar.middleware import DebugToolbarMiddleware from debug_toolbar.panels import DebugPanel from debug_toolbar.utils import render_stacktrace from debug_toolbar.utils.sql import reformat_sql from debug_toolbar.utils.tracking.db import CursorWrapper -from debug_toolbar.utils.tracking import replace_method - - -@replace_method(BaseDatabaseWrapper, 'cursor') -def cursor(original, self): - result = original(self) - - djdt = DebugToolbarMiddleware.get_current() - if not djdt: - return result - logger = djdt.get_panel(SQLDebugPanel) - - return CursorWrapper(result, self, logger=logger) def get_isolation_level_display(engine, level): @@ -131,6 +116,16 @@ def title(self): 'SQL Queries from %(count)d connections', count) % dict(count=count) + def enable_instrumentation(self): + # This is thread-safe because database connections are thread-local. + for connection in connections.all(): + old_cursor = connection.cursor + connection.cursor = lambda: CursorWrapper(old_cursor(), connection, self) + + def disable_instrumentation(self): + for connection in connections.all(): + del connection.cursor + def process_response(self, request, response): if self._queries: width_ratio_tally = 0 diff --git a/debug_toolbar/panels/template.py b/debug_toolbar/panels/template.py index 5bc0d2bd7..6523155cd 100644 --- a/debug_toolbar/panels/template.py +++ b/debug_toolbar/panels/template.py @@ -18,9 +18,9 @@ # Code taken and adapted from Simon Willison and Django Snippets: # http://www.djangosnippets.org/snippets/766/ -# Monkeypatch instrumented test renderer from django.test.utils - we could use -# django.test.utils.setup_test_environment for this but that would also set up -# e-mail interception, which we don't want +# Monkey-patch to enable the template_rendered signal. The receiver returns +# immediately when the panel is disabled to keep the overhead small. + from django.test.utils import instrumented_test_render from django.template import Template @@ -56,13 +56,17 @@ def __init__(self, *args, **kwargs): template_rendered.connect(self._store_template_info) def _store_template_info(self, sender, **kwargs): - t = kwargs['template'] - if t.name and t.name.startswith('debug_toolbar/'): - return # skip templates that we are generating through the debug toolbar. - context_data = kwargs['context'] + if not self.enabled: + return + + template, context = kwargs['template'], kwargs['context'] + + # Skip templates that we are generating through the debug toolbar. + if template.name and template.name.startswith('debug_toolbar/'): + return context_list = [] - for context_layer in context_data.dicts: + for context_layer in context.dicts: temp_layer = {} if hasattr(context_layer, 'items'): for key, value in context_layer.items(): @@ -102,6 +106,7 @@ def _store_template_info(self, sender, **kwargs): context_list.append(pformat(temp_layer)) except UnicodeEncodeError: pass + kwargs['context'] = [force_text(item) for item in context_list] self.templates.append(kwargs) diff --git a/tests/panels/test_sql.py b/tests/panels/test_sql.py index a1de0ea99..56b70dc72 100644 --- a/tests/panels/test_sql.py +++ b/tests/panels/test_sql.py @@ -17,6 +17,11 @@ class SQLPanelTestCase(BaseTestCase): def setUp(self): super(SQLPanelTestCase, self).setUp() self.panel = self.toolbar.get_panel(SQLDebugPanel) + self.panel.enable_instrumentation() + + def tearDown(self): + self.panel.disable_instrumentation() + super(SQLPanelTestCase, self).tearDown() def test_recording(self): self.assertEqual(len(self.panel._queries), 0) diff --git a/tests/panels/test_template.py b/tests/panels/test_template.py index c98521e6a..545ad9b56 100644 --- a/tests/panels/test_template.py +++ b/tests/panels/test_template.py @@ -18,6 +18,12 @@ class TemplateDebugPanelTestCase(BaseTestCase): def setUp(self): super(TemplateDebugPanelTestCase, self).setUp() self.panel = self.toolbar.get_panel(TemplateDebugPanel) + self.sql_panel = self.toolbar.get_panel(SQLDebugPanel) + self.sql_panel.enable_instrumentation() + + def tearDown(self): + self.sql_panel.disable_instrumentation() + super(TemplateDebugPanelTestCase, self).tearDown() def test_queryset_hook(self): t = Template("No context variables here!") @@ -30,8 +36,7 @@ def test_queryset_hook(self): t.render(c) # ensure the query was NOT logged - sql_panel = self.toolbar.get_panel(SQLDebugPanel) - self.assertEqual(len(sql_panel._queries), 0) + self.assertEqual(len(self.sql_panel._queries), 0) base_ctx_idx = 1 if django.VERSION[:2] >= (1, 5) else 0 ctx = self.panel.templates[0]['context'][base_ctx_idx] From b9d2abfeddaaea4c70f75e0c19a40195740b009d Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sun, 10 Nov 2013 16:02:24 +0100 Subject: [PATCH 3/4] Remove code that isn't used any more. --- debug_toolbar/middleware.py | 4 ---- debug_toolbar/utils/tracking/__init__.py | 19 ------------------- 2 files changed, 23 deletions(-) diff --git a/debug_toolbar/middleware.py b/debug_toolbar/middleware.py index 80e930508..daea751a7 100644 --- a/debug_toolbar/middleware.py +++ b/debug_toolbar/middleware.py @@ -54,10 +54,6 @@ class DebugToolbarMiddleware(object): """ debug_toolbars = {} - @classmethod - def get_current(cls): - return cls.debug_toolbars.get(threading.current_thread().ident) - def __init__(self): self._urlconfs = {} diff --git a/debug_toolbar/utils/tracking/__init__.py b/debug_toolbar/utils/tracking/__init__.py index a8a042151..e69de29bb 100644 --- a/debug_toolbar/utils/tracking/__init__.py +++ b/debug_toolbar/utils/tracking/__init__.py @@ -1,19 +0,0 @@ -from __future__ import unicode_literals - - -def replace_method(klass, method_name): - original = getattr(klass, method_name) - - def inner(callback): - def wrapped(*args, **kwargs): - return callback(original, *args, **kwargs) - - actual = getattr(original, '__wrapped__', original) - wrapped.__wrapped__ = actual - wrapped.__doc__ = getattr(actual, '__doc__', None) - wrapped.__name__ = actual.__name__ - - setattr(klass, method_name, wrapped) - return wrapped - - return inner From f5eaa8ebb0d1ca13aa2453ef88bb1eb223757716 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sun, 10 Nov 2013 15:59:30 +0100 Subject: [PATCH 4/4] Add test for the cache panel. --- tests/panels/test_cache.py | 28 ++++++++++++++++++++++++++++ tests/tests.py | 1 + 2 files changed, 29 insertions(+) create mode 100644 tests/panels/test_cache.py diff --git a/tests/panels/test_cache.py b/tests/panels/test_cache.py new file mode 100644 index 000000000..f3c359c18 --- /dev/null +++ b/tests/panels/test_cache.py @@ -0,0 +1,28 @@ +# coding: utf-8 + +from __future__ import unicode_literals + +from django.core import cache + +from debug_toolbar.panels.cache import CacheDebugPanel + +from ..base import BaseTestCase + + +class CachePanelTestCase(BaseTestCase): + + def setUp(self): + super(CachePanelTestCase, self).setUp() + self.panel = self.toolbar.get_panel(CacheDebugPanel) + self.panel.enable_instrumentation() + + def tearDown(self): + self.panel.disable_instrumentation() + super(CachePanelTestCase, self).tearDown() + + def test_recording(self): + self.assertEqual(len(self.panel.calls), 0) + cache.cache.set('foo', 'bar') + cache.cache.get('foo') + cache.cache.delete('foo') + self.assertEqual(len(self.panel.calls), 3) diff --git a/tests/tests.py b/tests/tests.py index e373d51e4..b92689541 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -2,6 +2,7 @@ if django.VERSION[:2] < (1, 6): # unittest-style discovery isn't available from .commands.test_debugsqlshell import * # noqa + from .panels.test_cache import * # noqa from .panels.test_logger import * # noqa from .panels.test_profiling import * # noqa from .panels.test_request_vars import * # noqa