Skip to content

Commit bc9def5

Browse files
pqCommit Queue
authored andcommitted
[analytics] track time in fix producer compute()s
Adds some insight into fix producer `compute` calls to the `GetFixes` diagnostics page. See updated screenshot in #60258. Bug: #60258 Change-Id: I5d8c1911e86ef02b8d543fcda367b3e9aa6544f6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/414341 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Phil Quitslund <[email protected]>
1 parent 9099999 commit bc9def5

File tree

5 files changed

+60
-10
lines changed

5 files changed

+60
-10
lines changed

pkg/analysis_server/lib/src/handler/legacy/edit_get_fixes.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ class EditGetFixesHandler extends LegacyHandler
212212
content: unitResult.content,
213213
offset: offset,
214214
requestLatency: peformanceTracker.computeTime!.inMilliseconds,
215+
producerTimings: peformanceTracker.producerTimings,
215216
),
216217
);
217218
} on InconsistentAnalysisException {

pkg/analysis_server/lib/src/lsp/handlers/code_actions/dart.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,6 @@ class DartCodeActionsProducer extends AbstractCodeActionsProducer {
186186
error: error,
187187
);
188188

189-
// TODO(pq): move this timer UP?
190189
var peformanceTracker = FixPerformance();
191190

192191
var fixes = await computeFixes(context, performance: peformanceTracker);
@@ -199,6 +198,7 @@ class DartCodeActionsProducer extends AbstractCodeActionsProducer {
199198
content: unitResult.content,
200199
offset: offset,
201200
requestLatency: peformanceTracker.computeTime!.inMilliseconds,
201+
producerTimings: peformanceTracker.producerTimings,
202202
),
203203
);
204204
}

pkg/analysis_server/lib/src/services/correction/fix_performance.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// BSD-style license that can be found in the LICENSE file.
44

55
import 'package:analysis_server/src/server/performance.dart';
6+
import 'package:analysis_server_plugin/src/correction/fix_processor.dart';
67

78
// TODO(pq): update to share w/ completion_performance
89
/// Compute a string representing a get fixes request at the
@@ -38,13 +39,15 @@ String _computeSourceSnippet(String contents, int offset) {
3839
class GetFixesPerformance extends RequestPerformance {
3940
final String path;
4041
final String snippet;
42+
final List<ProducerTiming> producerTimings;
4143

4244
GetFixesPerformance({
4345
required super.performance,
4446
required this.path,
4547
super.requestLatency,
4648
required String content,
4749
required int offset,
50+
required this.producerTimings,
4851
}) : snippet = _computeSourceSnippet(content, offset),
4952
super(operation: 'GetFixes');
5053

pkg/analysis_server/lib/src/status/diagnostics.dart

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1571,14 +1571,19 @@ class GetFixesPage extends DiagnosticPageWithNav with PerformanceChartMixin {
15711571

15721572
// Emit the data as a table
15731573
buf.writeln('<table>');
1574-
buf.writeln('<tr><th>Time</th><th>Source</th><th>Snippet</th></tr>');
1574+
buf.writeln(
1575+
'<tr><th>Time</th><th align = "left" title="Time in correction producer `compute()` calls">Producer.compute()</th><th align = "left">Source</th><th>Snippet</th></tr>',
1576+
);
1577+
15751578
for (var request in requests) {
15761579
var shortName = pathContext.basename(request.path);
1580+
var (producerTime, producerDetails) = _producerDetails(request);
15771581
buf.writeln(
15781582
'<tr>'
15791583
'<td class="pre right"><a href="/timing?id=${request.id}&kind=getFixes">'
15801584
'${_formatTiming(request)}'
15811585
'</a></td>'
1586+
'<td><abbr title="$producerDetails">${printMilliseconds(producerTime)}</abbr></td>'
15821587
'<td>${escape(shortName)}</td>'
15831588
'<td><code>${escape(request.snippet)}</code></td>'
15841589
'</tr>',
@@ -1601,6 +1606,24 @@ class GetFixesPage extends DiagnosticPageWithNav with PerformanceChartMixin {
16011606

16021607
return buffer.toString();
16031608
}
1609+
1610+
(int, String) _producerDetails(GetFixesPerformance request) {
1611+
var details = StringBuffer();
1612+
1613+
var totalProducerTime = 0;
1614+
var producerTimings = request.producerTimings;
1615+
1616+
for (var timing
1617+
in producerTimings.sortedBy((t) => t.elapsedTime).reversed) {
1618+
var producerTime = timing.elapsedTime;
1619+
totalProducerTime += producerTime;
1620+
details.write(timing.className);
1621+
details.write(': ');
1622+
details.writeln(printMilliseconds(producerTime));
1623+
}
1624+
1625+
return (totalProducerTime, details.toString());
1626+
}
16041627
}
16051628

16061629
class LspCapabilitiesPage extends DiagnosticPageWithNav {

pkg/analysis_server_plugin/lib/src/correction/fix_processor.dart

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,30 +15,42 @@ import 'package:analyzer_plugin/utilities/change_builder/conflicting_edit_except
1515
Future<List<Fix>> computeFixes(DartFixContext context,
1616
{FixPerformance? performance}) async {
1717
return [
18-
...await FixProcessor(context).compute(performance: performance),
18+
...await FixProcessor(context, performance: performance).compute(),
1919
...await FixInFileProcessor(context).compute(),
2020
];
2121
}
2222

23+
/// Timing information for a producer's call to `compute()`.
24+
typedef ProducerTiming = ({
25+
/// The producer class name.
26+
String className,
27+
28+
/// The time elapsed during `compute()`.
29+
int elapsedTime,
30+
});
31+
2332
/// A callback for recording fix request timings.
2433
class FixPerformance {
2534
Duration? computeTime;
35+
List<ProducerTiming> producerTimings = [];
2636
}
2737

2838
/// The computer for Dart fixes.
2939
class FixProcessor {
3040
final DartFixContext _fixContext;
41+
final FixPerformance? _performance;
42+
final Stopwatch _timer = Stopwatch();
3143

3244
final List<Fix> _fixes = <Fix>[];
3345

34-
FixProcessor(this._fixContext);
46+
FixProcessor(this._fixContext, {FixPerformance? performance})
47+
: _performance = performance;
3548

36-
Future<List<Fix>> compute({FixPerformance? performance}) async {
37-
var timer = Stopwatch();
38-
timer.start();
49+
Future<List<Fix>> compute() async {
50+
_timer.start();
3951
await _addFromProducers();
40-
timer.stop();
41-
performance?.computeTime = timer.elapsed;
52+
_timer.stop();
53+
_performance?.computeTime = _timer.elapsed;
4254
return _fixes;
4355
}
4456

@@ -74,7 +86,18 @@ class FixProcessor {
7486
ChangeBuilder(workspace: _fixContext.workspace, eol: producer.eol);
7587
try {
7688
var fixKind = producer.fixKind;
77-
await producer.compute(builder);
89+
90+
if (_performance != null) {
91+
var startTime = _timer.elapsedMilliseconds;
92+
await producer.compute(builder);
93+
_performance.producerTimings.add((
94+
className: producer.runtimeType.toString(),
95+
elapsedTime: _timer.elapsedMilliseconds - startTime,
96+
));
97+
} else {
98+
await producer.compute(builder);
99+
}
100+
78101
assert(
79102
!producer.canBeAppliedAcrossSingleFile || producer.fixKind == fixKind,
80103
'Producers used in bulk fixes must not modify the FixKind during '

0 commit comments

Comments
 (0)