Skip to content

Commit

Permalink
Use FastHash on Piece. (#1598)
Browse files Browse the repository at this point in the history
This mixin overrides the default identity-based hash code with a stored integer hash code. That made a big difference in the old formatter's performance but for most of the times I tried it with the new one, it didn't help. But it seems that the new formatter is storing pieces in maps enough for it to make a difference.

On the microbenchmarks:

```
Benchmark (tall)                fastest   median  slowest  average  baseline
-----------------------------  --------  -------  -------  -------  --------
block                             0.057    0.061    0.121    0.065    107.0%
chain                             0.524    0.539    0.579    0.541    117.1%
collection                        0.247    0.256    0.273    0.257    107.1%
collection_large                  1.539    1.559    1.621    1.562    106.0%
conditional                       0.060    0.061    0.079    0.062    107.0%
curry                             0.455    0.462    0.477    0.463    118.0%
ffi                               0.143    0.149    0.159    0.148    105.4%
flutter_popup_menu_test           0.503    0.514    0.541    0.516    115.0%
flutter_scrollbar_test            0.399    0.406    0.450    0.410    113.2%
function_call                     1.658    1.709    1.849    1.716    106.0%
infix_large                       0.552    0.567    0.600    0.570    108.9%
infix_small                       0.145    0.148    0.162    0.149    105.3%
interpolation                     0.087    0.089    0.101    0.090    102.5%
interpolation_1516                0.070    0.071    0.085    0.071    100.7%
large                             3.263    3.280    3.573    3.298    109.0%
top_level                         0.127    0.128    0.147    0.130    107.9%
```

And when formatting the Flutter repo:

```
Current formatter    10.138 ========================================
Optimized             9.490 =====================================
Old formatter         4.812 ==================
The current formatter is 52.54% slower than the old formatter.
The optimized is 6.83% faster than the current formatter.
The optimized is 49.29% slower than the old formatter.
The optimization gets the formatter 12.17% of the way to the old one.
```

That's pretty good for a one line change.

Also, this makes debugging the new formatter easier because now each Piece has a different debug string. Trying to figure out which of a hundred `Infix` pieces is the one you want gets pretty tedious...
  • Loading branch information
munificent authored Nov 13, 2024
1 parent ecfc0cb commit 02957e6
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 5 deletions.
File renamed without changes.
5 changes: 3 additions & 2 deletions lib/src/piece/piece.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.

import '../back_end/code_writer.dart';
import '../fast_hash.dart';
import '../profile.dart';

typedef Constrain = void Function(Piece other, State constrainedState);
Expand All @@ -14,7 +15,7 @@ typedef Constrain = void Function(Piece other, State constrainedState);
/// roughly follows the AST but includes comments and is optimized for
/// formatting and line splitting. The final output is then determined by
/// deciding which pieces split and how.
abstract base class Piece {
abstract base class Piece with FastHash {
/// The ordered list of all possible ways this piece could split.
///
/// Piece subclasses should override this if they support being split in
Expand Down Expand Up @@ -190,7 +191,7 @@ abstract base class Piece {
String get debugName => runtimeType.toString().replaceAll('Piece', '');

@override
String toString() => '$debugName${_pinnedState ?? ''}';
String toString() => '$debugName$id${_pinnedState ?? ''}';
}

/// A state that a piece can be in.
Expand Down
2 changes: 1 addition & 1 deletion lib/src/short/chunk.dart
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// Copyright (c) 2014, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import '../fast_hash.dart';
import '../profile.dart';
import 'fast_hash.dart';
import 'marking_scheme.dart';
import 'nesting_level.dart';
import 'rule/rule.dart';
Expand Down
2 changes: 1 addition & 1 deletion lib/src/short/nesting_level.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'fast_hash.dart';
import '../fast_hash.dart';
import 'marking_scheme.dart';

/// A single level of expression nesting.
Expand Down
2 changes: 1 addition & 1 deletion lib/src/short/rule/rule.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
// BSD-style license that can be found in the LICENSE file.

import '../../constants.dart';
import '../../fast_hash.dart';
import '../../profile.dart';
import '../chunk.dart';
import '../fast_hash.dart';

/// A constraint that determines the different ways a related set of chunks may
/// be split.
Expand Down

0 comments on commit 02957e6

Please sign in to comment.