Skip to content

[WebProfilerBundle] Improve cache panel #22129

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

[WebProfilerBundle] Improve cache panel #22129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d366d29
[WebProfilerBundle] Fix rendering empty cache calls
ro0NL Mar 23, 2017
39972cc
[WebProfilerBundle] Switch to tabs for cache panel
ro0NL Mar 23, 2017
40a6c75
improved metric formatting
ro0NL Mar 23, 2017
31d1933
improved/fixed profiler_dump in call table
ro0NL Mar 23, 2017
bc04a20
improves tabs top spacing
ro0NL Mar 24, 2017
c345e66
improved overall table
ro0NL Mar 24, 2017
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
153 changes: 86 additions & 67 deletions src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/cache.html.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
</div>
<div class="sf-toolbar-info-piece">
<b>Cache hits</b>
<span>{{ collector.totals.hits }}/{{ collector.totals.reads }} ({{ collector.totals['hits/reads'] }})</span>
<span>{{ collector.totals.hits }}/{{ collector.totals.reads }}{% if collector.totals.hit_read_ratio is not null %} ({{ collector.totals.hit_read_ratio }}%){% endif %}</span>
</div>
<div class="sf-toolbar-info-piece">
<b>Cache writes</b>
Expand Down Expand Up @@ -54,21 +54,14 @@
<span class="label">Total calls</span>
</div>
<div class="metric">
<span class="value">{{ '%0.2f'|format(collector.totals.time * 1000) }} ms</span>
<span class="value">{{ '%0.2f'|format(collector.totals.time * 1000) }} <span class="unit">ms</span></span>
<span class="label">Total time</span>
</div>
<div class="metric-divider"></div>
<div class="metric">
<span class="value">{{ collector.totals.reads }}</span>
<span class="label">Total reads</span>
</div>
<div class="metric">
<span class="value">{{ collector.totals.hits }}</span>
<span class="label">Total hits</span>
</div>
<div class="metric">
<span class="value">{{ collector.totals.misses }}</span>
<span class="label">Total misses</span>
</div>
<div class="metric">
<span class="value">{{ collector.totals.writes }}</span>
<span class="label">Total writes</span>
Expand All @@ -77,70 +70,96 @@
<span class="value">{{ collector.totals.deletes }}</span>
<span class="label">Total deletes</span>
</div>
<div class="metric-divider"></div>
<div class="metric">
<span class="value">{{ collector.totals.hits }}</span>
<span class="label">Total hits</span>
</div>
<div class="metric">
<span class="value">{{ collector.totals.misses }}</span>
<span class="label">Total misses</span>
</div>
<div class="metric">
<span class="value">{{ collector.totals['hits/reads'] }}</span>
<span class="value">
{% if collector.totals.hit_read_ratio is null %}
n/a
{% else %}
{{ collector.totals.hit_read_ratio }} <span class="unit">%</span>
{% endif %}
</span>
<span class="label">Hits/reads</span>
</div>
</div>

{% for name, calls in collector.calls %}
<h3>Statistics for '{{ name }}'</h3>
<div class="metrics">
{% for key, value in collector.statistics[name] %}
<div class="metric">
<span class="value">
{% if key == 'time' %}
{{ '%0.2f'|format(1000*value.value) }} ms
{% else %}
{{ value }}
{% endif %}
</span>
<span class="label">{{ key|capitalize }}</span>
</div>
{% endfor %}
</div>
<h4>Calls for '{{ name }}'</h4>
<h2>Pools</h2>
<div class="sf-tabs">
{% for name, calls in collector.calls %}
<div class="tab {{ calls|length == 0 ? 'disabled' }}">
<h3 class="tab-title">{{ name }} <span class="badge">{{ collector.statistics[name].calls }}</span></h3>

{% if not collector.totals.calls %}
<p>
<em>No calls.</em>
</p>
{% else %}
<table>
<thead>
<tr>
<th style="width: 5rem;">Key</th>
<th>Value</th>
</tr>
</thead>
<tbody>
{% for i, call in calls %}
<tr>
<th style="padding-top:2rem">#{{ i }}</th>
<th style="padding-top:2rem">Pool::{{ call.name }}</th>
</tr>
<tr>
<th>Argument</th>
<td>{{ profiler_dump(call.argument, maxDepth=2) }}</td>
</tr>
<tr>
<th>Results</th>
<td>
{% if call.result != false %}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Nyholm not sure about a) this check and b) loose comparison. As you can see in the screenshots null was hidden as wel. Does false need to be excluded, strictly?

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for this PR.
I'm not sure about your questions. Call.result could be false if there was an error (if I remember correctly). I think that is the reason for this check.

Why it not is a strict comparison... that is probably a typo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll check it out tonight. I'd prefer something like displaying n/a in that case, opposed to just an empty cell.

Copy link
Contributor Author

@ro0NL ro0NL Mar 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@Nyholm figured it out; this is (AFAIK) the direct result value from the method call passed by the traceableadapter.

Now given https://github.com/php-fig/cache/blob/master/src/CacheItemInterface.php#L42 i dont think we should maky any assumptions here.

Besides, this hides false as a result from a hasItem call ;-)

{{ profiler_dump(call.result, maxDepth=1) }}
<div class="tab-content">
<h3>Statistics</h3>
<div class="metrics">
{% for key, value in collector.statistics[name] %}
<div class="metric">
<span class="value">
{% if key == 'time' %}
{{ '%0.2f'|format(1000 * value.value) }} <span class="unit">ms</span>
{% elseif key == 'hit_read_ratio' %}
{% if value.value is null %}
n/a
{% else %}
{{ value }} <span class="unit">%</span>
{% endif %}
{% else %}
{{ value }}
{% endif %}
</span>
<span class="label">{{ key == 'hit_read_ratio' ? 'Hits/reads' : key|capitalize }}</span>
</div>
{% if key == 'time' or key == 'deletes' %}
<div class="metric-divider"></div>
{% endif %}
</td>
</tr>
<tr>
<th>Time</th>
<td>{{ '%0.2f'|format((call.end - call.start) * 1000) }} ms</td>
</tr>

{% endfor %}
</tbody>
{% endfor %}
</div>

</table>
{% endif %}
{% endfor %}
<h4>Calls</h4>
{% if calls|length == 0 %}
<div class="empty">
<p>No calls</p>
</div>
{% else %}
<table>
<thead>
<tr>
<th>#</th>
<th class="key">Key</th>
<th>Value</th>
</tr>
</thead>
<tbody>
{% for call in calls %}
{% set separatorStyle = not loop.first ? ' style="border-top-width: medium;"' : '' %}
<tr>
<td class="font-normal text-small text-muted nowrap" rowspan="3"{{ separatorStyle|raw }}>{{ loop.index }}</td>
<th{{ separatorStyle|raw }}>{{ call.name }}</th>
<td{{ separatorStyle|raw }}>{{ profiler_dump(call.value.argument, maxDepth=2) }}</td>
</tr>
<tr>
<th>Result</th>
<td>{{ profiler_dump(call.value.result, maxDepth=1) }}</td>
</tr>
<tr>
<th>Time</th>
<td>{{ '%0.2f'|format((call.end - call.start) * 1000) }} ms</td>
</tr>
{% endfor %}
</tbody>
</table>
{% endif %}
</div>
</div>
{% endfor %}
</div>

{% endblock %}
16 changes: 8 additions & 8 deletions src/Symfony/Component/Cache/DataCollector/CacheDataCollector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ private function calculateStatistics()
'calls' => 0,
'time' => 0,
'reads' => 0,
'hits' => 0,
'misses' => 0,
'writes' => 0,
'deletes' => 0,
'hits' => 0,
'misses' => 0,
);
/** @var TraceableAdapterEvent $call */
foreach ($calls as $call) {
Expand Down Expand Up @@ -138,9 +138,9 @@ private function calculateStatistics()
}
}
if ($statistics[$name]['reads']) {
$statistics[$name]['hits/reads'] = round(100 * $statistics[$name]['hits'] / $statistics[$name]['reads'], 2).'%';
$statistics[$name]['hit_read_ratio'] = round(100 * $statistics[$name]['hits'] / $statistics[$name]['reads'], 2);
} else {
$statistics[$name]['hits/reads'] = 'N/A';
$statistics[$name]['hit_read_ratio'] = null;
}
}

Expand All @@ -157,20 +157,20 @@ private function calculateTotalStatistics()
'calls' => 0,
'time' => 0,
'reads' => 0,
'hits' => 0,
'misses' => 0,
'writes' => 0,
'deletes' => 0,
'hits' => 0,
'misses' => 0,
);
foreach ($statistics as $name => $values) {
foreach ($totals as $key => $value) {
$totals[$key] += $statistics[$name][$key];
}
}
if ($totals['reads']) {
$totals['hits/reads'] = round(100 * $totals['hits'] / $totals['reads'], 2).'%';
$totals['hit_read_ratio'] = round(100 * $totals['hits'] / $totals['reads'], 2);
} else {
$totals['hits/reads'] = 'N/A';
$totals['hit_read_ratio'] = null;
}

return $totals;
Expand Down