From 02957e61b74a862051fb5e3fcce88ca66bab973b Mon Sep 17 00:00:00 2001 From: Bob Nystrom Date: Wed, 13 Nov 2024 11:24:03 -0800 Subject: [PATCH] Use FastHash on Piece. (#1598) 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... --- lib/src/{short => }/fast_hash.dart | 0 lib/src/piece/piece.dart | 5 +++-- lib/src/short/chunk.dart | 2 +- lib/src/short/nesting_level.dart | 2 +- lib/src/short/rule/rule.dart | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-) rename lib/src/{short => }/fast_hash.dart (100%) diff --git a/lib/src/short/fast_hash.dart b/lib/src/fast_hash.dart similarity index 100% rename from lib/src/short/fast_hash.dart rename to lib/src/fast_hash.dart diff --git a/lib/src/piece/piece.dart b/lib/src/piece/piece.dart index 348bb43d..e44cac13 100644 --- a/lib/src/piece/piece.dart +++ b/lib/src/piece/piece.dart @@ -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); @@ -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 @@ -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. diff --git a/lib/src/short/chunk.dart b/lib/src/short/chunk.dart index c37ed110..9af7b879 100644 --- a/lib/src/short/chunk.dart +++ b/lib/src/short/chunk.dart @@ -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'; diff --git a/lib/src/short/nesting_level.dart b/lib/src/short/nesting_level.dart index 0040233f..6260b533 100644 --- a/lib/src/short/nesting_level.dart +++ b/lib/src/short/nesting_level.dart @@ -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. diff --git a/lib/src/short/rule/rule.dart b/lib/src/short/rule/rule.dart index 393d48d7..62f48228 100644 --- a/lib/src/short/rule/rule.dart +++ b/lib/src/short/rule/rule.dart @@ -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.