Skip to content

Commit 0160142

Browse files
authored
Add some profiling instrumentation to the old formatter. (#1492)
This helps comparing the different phases in the two implementations against each other. Also, I added support to benchmark/directory.dart to run in tall or short style.
1 parent 6641bde commit 0160142

File tree

8 files changed

+49
-9
lines changed

8 files changed

+49
-9
lines changed

benchmark/directory.dart

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import 'package:dart_style/src/constants.dart';
1111
import 'package:dart_style/src/profile.dart';
1212
import 'package:dart_style/src/testing/benchmark.dart';
1313

14+
/// Whether to use the short or tall style formatter.
15+
bool _isShort = false;
16+
1417
/// Reads a given directory of Dart files and repeatedly formats their contents.
1518
///
1619
/// This allows getting profile information on a large real-world codebase
@@ -60,7 +63,8 @@ void main(List<String> arguments) async {
6063

6164
void _runFormatter(String source) {
6265
try {
63-
var formatter = DartFormatter(experimentFlags: [tallStyleExperimentFlag]);
66+
var formatter = DartFormatter(
67+
experimentFlags: [if (!_isShort) tallStyleExperimentFlag]);
6468

6569
var result = formatter.format(source);
6670

@@ -77,6 +81,10 @@ void _runFormatter(String source) {
7781
Future<String> _parseArguments(List<String> arguments) async {
7882
var argParser = ArgParser();
7983
argParser.addFlag('help', negatable: false, help: 'Show usage information.');
84+
argParser.addFlag('short',
85+
abbr: 's',
86+
negatable: false,
87+
help: 'Whether the formatter should use short or tall style.');
8088
argParser.addFlag('aot',
8189
negatable: false,
8290
help: 'Whether the benchmark should run in AOT mode versus JIT.');
@@ -93,9 +101,14 @@ Future<String> _parseArguments(List<String> arguments) async {
93101
}
94102

95103
if (argResults['aot'] as bool) {
96-
await rerunAsAot([argResults.rest.single]);
104+
await rerunAsAot([
105+
for (var argument in arguments)
106+
if (argument != '--aot') argument,
107+
]);
97108
}
98109

110+
_isShort = argResults['short'] as bool;
111+
99112
return argResults.rest.single;
100113
}
101114

lib/src/back_end/solver.dart

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ class Solver {
8686
rootState: rootState);
8787

8888
Profile.begin('Solver enqueue');
89+
Profile.count('Solver enqueue');
8990
_queue.add(solution);
9091
Profile.end('Solver enqueue');
9192

@@ -95,9 +96,8 @@ class Solver {
9596
var attempts = 0;
9697

9798
while (_queue.isNotEmpty && attempts < _maxAttempts) {
98-
Profile.begin('Solver dequeue');
99+
Profile.count('Solver dequeue');
99100
var solution = _queue.removeFirst();
100-
Profile.end('Solver dequeue');
101101

102102
attempts++;
103103

@@ -125,9 +125,8 @@ class Solver {
125125
// options.
126126
for (var expanded in solution.expand(_cache, root,
127127
pageWidth: _pageWidth, leadingIndent: _leadingIndent)) {
128-
Profile.begin('Solver enqueue');
128+
Profile.count('Solver enqueue');
129129
_queue.add(expanded);
130-
Profile.end('Solver enqueue');
131130
}
132131
}
133132

lib/src/short/chunk.dart

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) 2014, the Dart project authors. Please see the AUTHORS file
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
4+
import '../profile.dart';
45
import 'fast_hash.dart';
56
import 'marking_scheme.dart';
67
import 'nesting_level.dart';
@@ -100,7 +101,9 @@ class Chunk extends Selection {
100101
: _text = '',
101102
_flushLeft = flushLeft,
102103
_isDouble = isDouble,
103-
_spaceWhenUnsplit = space;
104+
_spaceWhenUnsplit = space {
105+
Profile.count('Create Chunk');
106+
}
104107

105108
/// Creates a dummy chunk.
106109
///
@@ -113,7 +116,9 @@ class Chunk extends Selection {
113116
nesting = NestingLevel(),
114117
_spaceWhenUnsplit = false,
115118
_flushLeft = false,
116-
_isDouble = false;
119+
_isDouble = false {
120+
Profile.count('Create Chunk');
121+
}
117122

118123
/// Append [text] to the end of the chunk's text.
119124
void appendText(String text) {

lib/src/short/chunk_builder.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import '../comment_type.dart';
77
import '../constants.dart';
88
import '../dart_formatter.dart';
99
import '../debug.dart' as debug;
10+
import '../profile.dart';
1011
import '../source_code.dart';
1112
import 'chunk.dart';
1213
import 'line_writer.dart';
@@ -653,10 +654,14 @@ class ChunkBuilder {
653654
debug.log();
654655
}
655656

657+
Profile.begin('ChunkBuilder run line splitter');
658+
656659
var writer = LineWriter(_formatter, _chunks);
657660
var result =
658661
writer.writeLines(isCompilationUnit: _source.isCompilationUnit);
659662

663+
Profile.end('ChunkBuilder run line splitter');
664+
660665
int? selectionStart;
661666
int? selectionLength;
662667
if (_source.selectionStart != null) {

lib/src/short/line_splitting/solve_state.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
import '../../constants.dart';
55
import '../../debug.dart' as debug;
6+
import '../../profile.dart';
67
import '../chunk.dart';
78
import '../nesting_level.dart';
89
import '../rule/rule.dart';
@@ -108,6 +109,8 @@ class SolveState {
108109
_initBoundRulesInUnboundLines();
109110

110111
SolveState(this._splitter, this._ruleValues) {
112+
Profile.count('Create SolveState');
113+
111114
_calculateSplits();
112115
_calculateCost();
113116
}

lib/src/short/line_splitting/solve_state_queue.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
4+
import '../../profile.dart';
45
import 'line_splitter.dart';
56
import 'solve_state.dart';
67

@@ -39,6 +40,8 @@ class SolveStateQueue {
3940
///
4041
/// Grows the capacity if the backing list is full.
4142
void add(SolveState state) {
43+
Profile.count('SolveStateQueue.add()');
44+
4245
if (_tryOverlap(state)) return;
4346

4447
if (_length == _queue.length) {
@@ -56,6 +59,8 @@ class SolveStateQueue {
5659
SolveState removeFirst() {
5760
assert(_length > 0);
5861

62+
Profile.count('SolveStateQueue.removeFirst()');
63+
5964
// Remove the highest priority state.
6065
var result = _queue[0]!;
6166
_length--;

lib/src/short/rule/rule.dart

Lines changed: 6 additions & 1 deletion
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 '../../constants.dart';
6+
import '../../profile.dart';
67
import '../chunk.dart';
78
import '../fast_hash.dart';
89

@@ -80,10 +81,14 @@ class Rule extends FastHash {
8081
/// rules.
8182
bool get splitsOnInnerRules => true;
8283

83-
Rule([this._cost = Cost.normal]);
84+
Rule([this._cost = Cost.normal]) {
85+
Profile.count('Create Rule');
86+
}
8487

8588
/// Creates a new rule that is already fully split.
8689
Rule.hard() : _cost = 0 {
90+
Profile.count('Create Rule');
91+
8792
// Set the cost to zero since it will always be applied, so there's no
8893
// point in penalizing it.
8994
//

lib/src/short/source_visitor.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import '../ast_extensions.dart';
1414
import '../comment_type.dart';
1515
import '../constants.dart';
1616
import '../dart_formatter.dart';
17+
import '../profile.dart';
1718
import '../source_code.dart';
1819
import 'argument_list_visitor.dart';
1920
import 'call_chain_visitor.dart';
@@ -119,13 +120,17 @@ class SourceVisitor extends ThrowingAstVisitor {
119120
/// This is the only method that should be called externally. Everything else
120121
/// is effectively private.
121122
SourceCode run(AstNode node) {
123+
Profile.begin('SourceVisitor create Chunks');
124+
122125
visit(node);
123126

124127
// Output trailing comments.
125128
writePrecedingCommentsAndNewlines(node.endToken.next!);
126129

127130
assert(_constNesting == 0, 'Should have exited all const contexts.');
128131

132+
Profile.end('SourceVisitor create Chunks');
133+
129134
// Finish writing and return the complete result.
130135
return builder.end();
131136
}

0 commit comments

Comments
 (0)