-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Download prof files #2286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Download prof files #2286
Changes from all commits
48ac250
49f29eb
7ec7f25
fe4e305
5db558d
74ddf5e
5bbb211
a285d82
2bd5cfe
e698cde
66e2d50
3896fe3
09f2ba0
9829c58
cfc244b
df258a2
efbd2b9
9cbdf04
05a505c
34dde67
bdb6bf5
776e4ba
25fa939
f1c86c2
d03fec3
7235ab8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,26 @@ | ||
| import cProfile | ||
| import logging | ||
| import os | ||
| from colorsys import hsv_to_rgb | ||
| from pstats import Stats | ||
|
|
||
| from django import forms | ||
| from django.conf import settings | ||
| from django.contrib.auth.decorators import login_not_required | ||
| from django.http import FileResponse, Http404, HttpResponseBadRequest | ||
| from django.urls import path | ||
| from django.utils import timezone | ||
| from django.utils.html import format_html | ||
| from django.utils.translation import gettext_lazy as _ | ||
| from django.views.decorators.http import require_GET | ||
|
|
||
| from debug_toolbar import settings as dt_settings | ||
| from debug_toolbar.decorators import render_with_toolbar_language, require_show_toolbar | ||
| from debug_toolbar.panels import Panel | ||
| from debug_toolbar.toolbar import DebugToolbar | ||
| from debug_toolbar.utils import ViewPathMetadata | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class FunctionCall: | ||
|
|
@@ -131,22 +143,69 @@ def cumtime_per_call(self): | |
| def indent(self): | ||
| return 16 * self.depth | ||
|
|
||
| def primitive_count(self): | ||
| return self.stats[0] | ||
|
|
||
| def _func_key(self): | ||
| # Mirrors pstats.func_std_string to produce the plain-text identifier | ||
| # used in downloaded .prof files. | ||
| # https://docs.python.org/3/library/profile.html#pstats.Stats | ||
| if self.func[:2] == ("~", 0): | ||
| name = self.func[2] | ||
| if name.startswith("<") and name.endswith(">"): | ||
| return f"{{{name[1:-1]}}}" | ||
| return name | ||
| return "%s:%d(%s)" % self.func | ||
|
|
||
| def serialize(self): | ||
| return { | ||
| "has_subfuncs": self.has_subfuncs, | ||
| "id": self.id, | ||
| "parent_ids": self.parent_ids, | ||
| "is_project_func": self.is_project_func(), | ||
| "indent": self.indent(), | ||
| "func_key": self._func_key(), | ||
| "func_std_string": self.func_std_string(), | ||
| "cumtime": self.cumtime(), | ||
| "cumtime_per_call": self.cumtime_per_call(), | ||
| "tottime": self.tottime(), | ||
| "tottime_per_call": self.tottime_per_call(), | ||
| "count": self.count(), | ||
| "primitive_count": self.primitive_count(), | ||
| } | ||
|
|
||
|
|
||
| def _print_stats_from_function_calls(func_list: list[FunctionCall]) -> str: | ||
| """ | ||
| Create a string that is matches what pstats.Stats.print_stats would | ||
| print out. | ||
| """ | ||
| # Column layout matches pstats output: ncalls right-justified in 9 | ||
| # chars, each time value formatted as f8 ("%8.3f"). The 3-space indent | ||
| # on the header and ordering lines is part of the pstats format. | ||
| # https://docs.python.org/3/library/profile.html#pstats.Stats.print_stats | ||
|
Comment on lines
+183
to
+186
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't want to insinuate anything, but the comments read like AI code. Maybe we don't fan the fire and limit comments to only what can't be easily understood from the code itself.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I called out that I used Claude in the PR. I think in this case the comments help. There's syntax used in this function that isn't typical in my opinion, so having it explained helps.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, sorry. I only read that comment later. Hm… depends. I think tests for "how this should look" actually help me personally more than the comments. But better too much documentation that too little 👍 At least my spidysense for AI still works. 🤠 |
||
| lines = [" Ordered by: cumulative time", ""] | ||
| lines.append( | ||
| " ncalls tottime percall cumtime percall filename:lineno(function)" | ||
| ) | ||
| for func in func_list: | ||
| nc = func["count"] | ||
| cc = func["primitive_count"] | ||
| # pstats shows nc/cc when they differ (recursive calls inflate nc) | ||
| ncalls_str = f"{nc}/{cc}" if nc != cc else str(nc) | ||
| # pstats prints 8 spaces instead of a percall value when the | ||
| # divisor is zero, preserving column alignment | ||
| tt_per = f"{func['tottime_per_call']:8.3f}" if nc else " " * 8 | ||
| ct_per = f"{func['cumtime_per_call']:8.3f}" if cc else " " * 8 | ||
| lines.append( | ||
| f"{ncalls_str:>9} {func['tottime']:8.3f} {tt_per}" | ||
| f" {func['cumtime']:8.3f} {ct_per} {func['func_key']}" | ||
| ) | ||
| # pstats ends output with two blank lines | ||
| lines.extend(["", ""]) | ||
| return "\n".join(lines) | ||
|
|
||
|
|
||
| class ProfilingPanel(Panel): | ||
| """ | ||
| Panel that displays profiling information. | ||
|
|
@@ -184,7 +243,6 @@ def generate_stats(self, request, response): | |
| self.stats.calc_callees() | ||
|
|
||
| root_func = cProfile.label(super().process_request.__code__) | ||
|
|
||
| if root_func in self.stats.stats: | ||
| root = FunctionCall(self.stats, root_func, depth=0) | ||
| func_list = [] | ||
|
|
@@ -197,4 +255,67 @@ def generate_stats(self, request, response): | |
| dt_settings.get_config()["PROFILER_MAX_DEPTH"], | ||
| cum_time_threshold, | ||
| ) | ||
| self.record_stats({"func_list": [func.serialize() for func in func_list]}) | ||
| try: | ||
| url_name = ViewPathMetadata.from_request(request).url_name | ||
| except Http404: | ||
| url_name = _("<unavailable>") | ||
| self.record_stats( | ||
| { | ||
| "func_list": [func.serialize() for func in func_list], | ||
| "profile_name": f"{url_name}-{timezone.now().isoformat()}", | ||
| } | ||
| ) | ||
|
|
||
| def get_stats(self): | ||
| """ | ||
| Access data stored by the panel. Returns a :class:`dict`. | ||
| """ | ||
| stats = super().get_stats() | ||
| # This isn't a stat, but is used in the context in the | ||
| # ProfilingPanel.content property when rendering the template. | ||
| stats["profiling_download_form"] = ProfilingDownloadForm( | ||
| initial={"request_id": self.toolbar.request_id} | ||
| ) | ||
| return stats | ||
|
|
||
| @classmethod | ||
| def get_urls(cls): | ||
| return [ | ||
| path("profiling_download/", profiling_download, name="profiling_download") | ||
| ] | ||
|
|
||
|
|
||
| class ProfilingDownloadForm(forms.Form): | ||
| """ | ||
| Validate params | ||
|
|
||
| request_id: The key for the store instance to be fetched. | ||
| """ | ||
|
|
||
| request_id = forms.CharField(widget=forms.HiddenInput()) | ||
|
|
||
|
|
||
| @require_GET | ||
| @login_not_required | ||
| @require_show_toolbar | ||
| @render_with_toolbar_language | ||
| def profiling_download(request): | ||
| form = ProfilingDownloadForm(request.GET) | ||
|
|
||
| if form.is_valid(): | ||
| request_id: str = form.cleaned_data["request_id"] | ||
| toolbar: DebugToolbar | None = DebugToolbar.fetch(request_id) | ||
| if toolbar is None: | ||
| return HttpResponseBadRequest("Request is no longer available.") | ||
| panel = toolbar.get_panel_by_id(ProfilingPanel.panel_id) | ||
| panel_stats = panel.get_stats() | ||
| if "func_list" not in panel_stats: | ||
| return HttpResponseBadRequest("No profiling data exists for this request.") | ||
| content = _print_stats_from_function_calls(panel_stats["func_list"]) | ||
| return FileResponse( | ||
| content, | ||
| as_attachment=True, | ||
| filename=f"{panel_stats['profile_name']}.prof", | ||
| content_type="text/plain", | ||
| ) | ||
| return HttpResponseBadRequest(f"Form errors: {form.errors}") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,10 @@ | ||
| {% load i18n %} | ||
|
|
||
| <form method="get" action="{% url 'djdt:profiling_download' %}" target="_blank"> | ||
| {{ profiling_download_form.as_div }} | ||
| <button>Download</button> | ||
| </form> | ||
|
Comment on lines
+3
to
+6
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why use a form and a button for a GET request? Why not use an a-tag with a
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did it for consistency with the history panel which was originally done because the form used to inherit from SignedDataForm. I do like that it enforces the data validation more strictly.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which currently is only an ID, right? I don't really care much, but without knowing the history, I just looked a little odd.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, yup. That's correct. |
||
|
|
||
| <table> | ||
| <thead> | ||
| <tr> | ||
|
|
@@ -13,22 +19,22 @@ | |
| <tbody> | ||
| {% for call in func_list %} | ||
| <tr class="djdt-profile-row {% if call.is_project_func %}djdt-highlighted {% endif %} {% for parent_id in call.parent_ids %} djToggleDetails_{{ parent_id }}{% endfor %}" id="profilingMain_{{ call.id }}"> | ||
| <td> | ||
| <div data-djdt-styles="paddingLeft:{{ call.indent }}px"> | ||
| {% if call.has_subfuncs %} | ||
| <td> | ||
| <div data-djdt-styles="paddingLeft:{{ call.indent }}px"> | ||
| {% if call.has_subfuncs %} | ||
| <button type="button" class="djProfileToggleDetails djToggleSwitch" data-toggle-name="profilingMain" data-toggle-id="{{ call.id }}">-</button> | ||
| {% else %} | ||
| <span class="djNoToggleSwitch"></span> | ||
| {% endif %} | ||
| <span class="djdt-stack">{{ call.func_std_string|safe }}</span> | ||
| </div> | ||
| </td> | ||
| <td>{{ call.cumtime|floatformat:3 }}</td> | ||
| <td>{{ call.cumtime_per_call|floatformat:3 }}</td> | ||
| <td>{{ call.tottime|floatformat:3 }}</td> | ||
| <td>{{ call.tottime_per_call|floatformat:3 }}</td> | ||
| <td>{{ call.count }}</td> | ||
| </tr> | ||
| {% else %} | ||
| <span class="djNoToggleSwitch"></span> | ||
| {% endif %} | ||
| <span class="djdt-stack">{{ call.func_std_string|safe }}</span> | ||
| </div> | ||
| </td> | ||
| <td>{{ call.cumtime|floatformat:3 }}</td> | ||
| <td>{{ call.cumtime_per_call|floatformat:3 }}</td> | ||
| <td>{{ call.tottime|floatformat:3 }}</td> | ||
| <td>{{ call.tottime_per_call|floatformat:3 }}</td> | ||
| <td>{{ call.count }}</td> | ||
| </tr> | ||
| {% endfor %} | ||
| </tbody> | ||
| </table> | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this. I could use a second opinion on whether we want to go this approach or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pstats.Statscan be printed to any stream-like object. So you can get aio.StringIOand simply callprint_stats. I don't believe the is a need to recreate the funcitonality, right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The challenge is the panel doesn't show the same content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The profiling panel as a whole needs a bit of a rework. The threshold concept just doesn't seem to work properly. It seems like we should lean more into the actual APIs of the standard library.
Getting this all resolved is on my todo list, but I'd really like to avoid it for this feature.