Skip to content

Commit e7ce3a2

Browse files
authored
Optimize certain operations (#1206)
* _compareScore change * Mark spans in _calculateCost instead of using set * Mark NestingLevels in _calculateSplits instead of using set * Missing file and renames * Change and add comments * Feedback
1 parent 6438ee9 commit e7ce3a2

File tree

5 files changed

+64
-19
lines changed

5 files changed

+64
-19
lines changed

lib/src/chunk.dart

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
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.
44
import 'fast_hash.dart';
5+
import 'marking_scheme.dart';
56
import 'nesting_level.dart';
67
import 'rule/rule.dart';
78

@@ -294,7 +295,11 @@ class OpenSpan {
294295
/// This is a wrapper around the cost so that spans have unique identities.
295296
/// This way we can correctly avoid paying the cost multiple times if the same
296297
/// span is split by multiple chunks.
297-
class Span extends FastHash {
298+
///
299+
/// Spans can be marked during processing in an algorithm but should be left
300+
/// unmarked when the algorithm finishes to make marking work in subsequent
301+
/// calls.
302+
class Span extends FastHash with Markable {
298303
/// The cost applied when the span is split across multiple lines or `null`
299304
/// if the span is for a multisplit.
300305
final int cost;

lib/src/line_splitting/solve_state.dart

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -272,17 +272,23 @@ class SolveState {
272272
void _calculateSplits() {
273273
// Figure out which expression nesting levels got split and need to be
274274
// assigned columns.
275-
var usedNestingLevels = <NestingLevel>{};
275+
var usedNestingLevels = <NestingLevel>[];
276276
for (var i = 0; i < _splitter.chunks.length; i++) {
277277
var chunk = _splitter.chunks[i];
278278
if (chunk.rule.isSplit(getValue(chunk.rule), chunk)) {
279-
usedNestingLevels.add(chunk.nesting);
280-
chunk.nesting.clearTotalUsedIndent();
279+
var nesting = chunk.nesting;
280+
if (nesting.mark()) {
281+
usedNestingLevels.add(nesting);
282+
nesting.clearTotalUsedIndent();
283+
}
281284
}
282285
}
283286

284287
for (var nesting in usedNestingLevels) {
285-
nesting.refreshTotalUsedIndent(usedNestingLevels);
288+
nesting.refreshTotalUsedIndent();
289+
}
290+
for (var nesting in usedNestingLevels) {
291+
nesting.unmark();
286292
}
287293

288294
_splits = SplitSet(_splitter.chunks.length);
@@ -340,10 +346,11 @@ class SolveState {
340346
start = end;
341347
}
342348

343-
// The set of spans that contain chunks that ended up splitting. We store
344-
// these in a set so a span's cost doesn't get double-counted if more than
345-
// one split occurs in it.
346-
var splitSpans = <Span>{};
349+
// The list of spans that contain chunks that ended up splitting. These are
350+
// made unique by marking the spans during the run, adding them to this list
351+
// to be able to unmark them again. We have to keep track of uniqueness to
352+
// avoid double-counting if more than one split occurs in it.
353+
var splitSpans = <Span>[];
347354

348355
// The nesting level of the chunk that ended the previous line.
349356
NestingLevel? previousNesting;
@@ -354,7 +361,12 @@ class SolveState {
354361
if (_splits.shouldSplitAt(i)) {
355362
endLine(i);
356363

357-
splitSpans.addAll(chunk.spans);
364+
for (var span in chunk.spans) {
365+
if (span.mark()) {
366+
splitSpans.add(span);
367+
cost += span.cost;
368+
}
369+
}
358370

359371
// Do not allow sequential lines to have the same indentation but for
360372
// different reasons. In other words, don't allow different expressions
@@ -408,9 +420,9 @@ class SolveState {
408420
if (value != Rule.unsplit) cost += rule.cost;
409421
});
410422

411-
// Add the costs for the spans containing splits.
423+
// Unmark spans again so they're ready for another run.
412424
for (var span in splitSpans) {
413-
cost += span.cost;
425+
span.unmark();
414426
}
415427

416428
// Finish the last line.

lib/src/line_splitting/solve_state_queue.dart

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,11 @@ class SolveStateQueue {
8888

8989
/// Compares the overflow and cost of [a] to [b].
9090
int _compareScore(SolveState a, SolveState b) {
91-
if (a.splits.cost != b.splits.cost) {
92-
return a.splits.cost.compareTo(b.splits.cost);
91+
var aCost = a.splits.cost;
92+
var bCost = b.splits.cost;
93+
if (aCost != bCost) {
94+
if (aCost < bCost) return -1;
95+
return 1;
9396
}
9497

9598
return a.overflowChars.compareTo(b.overflowChars);

lib/src/marking_scheme.dart

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
/// A mixin for marking classes.
6+
mixin Markable {
7+
bool _isMarked = false;
8+
9+
bool mark() {
10+
if (_isMarked) return false;
11+
_isMarked = true;
12+
return true;
13+
}
14+
15+
bool get isMarked => _isMarked;
16+
17+
void unmark() {
18+
_isMarked = false;
19+
}
20+
}

lib/src/nesting_level.dart

Lines changed: 10 additions & 5 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 'fast_hash.dart';
6+
import 'marking_scheme.dart';
67

78
/// A single level of expression nesting.
89
///
@@ -19,7 +20,11 @@ import 'fast_hash.dart';
1920
/// indented relative to the outer expression. It's almost always
2021
/// [Indent.expression], but cascades are special magic snowflakes and use
2122
/// [Indent.cascade].
22-
class NestingLevel extends FastHash {
23+
///
24+
/// NestingLEvels can be marked during processing in an algorithm but should be
25+
/// left unmarked when the algorithm finishes to make marking work in subsequent
26+
/// calls.
27+
class NestingLevel extends FastHash with Markable {
2328
/// The nesting level surrounding this one, or `null` if this is represents
2429
/// top level code in a block.
2530
final NestingLevel? parent;
@@ -56,19 +61,19 @@ class NestingLevel extends FastHash {
5661
}
5762

5863
/// Calculates the total amount of indentation from this nesting level and
59-
/// all of its parents assuming only [usedNesting] levels are in use.
60-
void refreshTotalUsedIndent(Set<NestingLevel> usedNesting) {
64+
/// all of its parents assuming only marked levels are in use.
65+
void refreshTotalUsedIndent() {
6166
var totalIndent = _totalUsedIndent;
6267
if (totalIndent != null) return;
6368

6469
totalIndent = 0;
6570

6671
if (parent != null) {
67-
parent!.refreshTotalUsedIndent(usedNesting);
72+
parent!.refreshTotalUsedIndent();
6873
totalIndent += parent!.totalUsedIndent;
6974
}
7075

71-
if (usedNesting.contains(this)) totalIndent += indent;
76+
if (isMarked) totalIndent += indent;
7277

7378
_totalUsedIndent = totalIndent;
7479
}

0 commit comments

Comments
 (0)