Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions debug_toolbar/panels/timer.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ def content(self):
(_("System CPU time"), _("%(stime)0.3f ms") % stats),
(_("Total CPU time"), _("%(total)0.3f ms") % stats),
(_("Elapsed time"), _("%(total_time)0.3f ms") % stats),
(
_("Toolbar time"),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Toolbar is ambiguous outside this repo. Maybe stick with DebugToolbar, like it's currently phrased in most places.

Suggested change
_("Toolbar time"),
_("DebugToolbar time"),

_("%(toolbar_time)0.3f ms") % stats,
),
(
_("Context switches"),
_("%(vcsw)d voluntary, %(ivcsw)d involuntary") % stats,
Expand Down Expand Up @@ -109,6 +113,9 @@ def generate_stats(self, request, response):
# stats['urss'] = self._end_rusage.ru_idrss
# stats['usrss'] = self._end_rusage.ru_isrss

if hasattr(self, "_toolbar_start_time"):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EAFP: Maybe consider catching the AttributeError instead of the default if-branch.

stats["toolbar_time"] = (perf_counter() - self._toolbar_start_time) * 1000
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider storing the duration with an explicit type using datetime.timedelta. It's always easier than remembering that toolbar_time is in milliseconds.


self.record_stats(stats)

def generate_server_timing(self, request, response):
Expand All @@ -120,7 +127,16 @@ def generate_server_timing(self, request, response):
self.record_server_timing(
"total_time", "Elapsed time", stats.get("total_time", 0)
)
self.record_server_timing(
"toolbar_time", "Toolbar time", stats.get("toolbar_time", 0)
)

@staticmethod
def _elapsed_ru(start, end, name):
return end.get(name) - start.get(name)

def enable_instrumentation(self):
self._toolbar_start_time = perf_counter()

async def aenable_instrumentation(self):
self._toolbar_start_time = perf_counter()
Comment on lines +138 to +142
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both are synchronous, and instrumentation usually lives in the CPU and has no IO. Thus, I don't think asyncIO is all that helpful even in the future.

10 changes: 9 additions & 1 deletion debug_toolbar/toolbar.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,15 @@ def enabled_panels(self) -> list[Panel]:
"""
Get a list of panels enabled for the current request.
"""
return [panel for panel in self._panels.values() if panel.enabled]
panels = [panel for panel in self._panels.values() if panel.enabled]
# Ensure TimerPanel is first in order to measure the full time of the toolbar's processing.
timer_panel = next(
(panel for panel in panels if panel.panel_id == "TimerPanel"), None
)
Comment on lines +76 to +80
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are looping over the panels twice, O(n)=2*n. I think with a little rearrangement, this can be done in a single loop.

if timer_panel:
Comment on lines +78 to +81
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great place for a walrus:

Suggested change
timer_panel = next(
(panel for panel in panels if panel.panel_id == "TimerPanel"), None
)
if timer_panel:
if timer_panel := next(
(panel for panel in panels if panel.panel_id == "TimerPanel"), None
)

panels.remove(timer_panel)
panels.insert(0, timer_panel)
Comment on lines +82 to +83
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe replace the item instead of the stack operation.

Suggested change
panels.remove(timer_panel)
panels.insert(0, timer_panel)
panels[0] = timer_panel

return panels

@property
def csp_nonce(self) -> str | None:
Expand Down
1 change: 1 addition & 0 deletions docs/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ Pending
* Fixed a crash which occurred when using non-``str`` static file values.
* Documented experimental async support.
* Improved troubleshooting doc for incorrect mime types for .js static files
* Added toolbar time to the timer panel.

Please see everything under 5.0.0-alpha as well.

Expand Down
5 changes: 5 additions & 0 deletions tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,10 @@ def test_concurrent_async_sql_page(self):
len(response.toolbar.get_panel_by_id("SQLPanel").get_stats()["queries"]), 2
)

def test_timer_panel_first(self):
toolbar = DebugToolbar(self.request, self.get_response)
self.assertEqual(toolbar.enabled_panels[0].panel_id, "TimerPanel")


@override_settings(DEBUG=True)
class DebugToolbarIntegrationTestCase(IntegrationTestCase):
Expand Down Expand Up @@ -620,6 +624,7 @@ def test_server_timing_headers(self):
r'TimerPanel_stime;dur=(\d)*(\.(\d)*)?;desc="System CPU time", ',
r'TimerPanel_total;dur=(\d)*(\.(\d)*)?;desc="Total CPU time", ',
r'TimerPanel_total_time;dur=(\d)*(\.(\d)*)?;desc="Elapsed time", ',
r'TimerPanel_toolbar_time;dur=(\d)*(\.(\d)*)?;desc="Toolbar time", ',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Toolbar is ambiguous outside this repo. Maybe stick with DebugToolbar, like it's currently phrased in most places.

Suggested change
r'TimerPanel_toolbar_time;dur=(\d)*(\.(\d)*)?;desc="Toolbar time", ',
r'TimerPanel_dj_toolbar_time;dur=(\d)*(\.(\d)*)?;desc="DebugToolbar time", ',

r'SQLPanel_sql_time;dur=(\d)*(\.(\d)*)?;desc="SQL 1 queries", ',
r'CachePanel_total_time;dur=0;desc="Cache 0 Calls"',
]
Expand Down
Loading