Skip to content

Commit cd0f2ab

Browse files
munificentmunificent
authored
Solve subtrees separately (#1376)
Support formatting Piece subtrees separately from the surrounding Piece. This is the biggest optimization hammer because it takes a problem that is combinatorial in the total number of Pieces in the tree and breaks the Piece tree into separate subtrees that can each be formatted independently. Further, it allows reusing subtree Solutions across different Solutions for the surrounding Piece trees. However, knowing when this optimization can be applied is tricky. The whole reason that solving is combinatorial is because in many regions of code, we want to find the globally optimal solution and Pieces are interdependent since how one Piece formats determines where in the current line the next Piece starts, how indented it is, possibly other constraints, etc. I tried but wasn't able to find a robust automatic way for CodeWriter to tell when formatting a given child Piece can be done independently. Instead, we rely on each Piece to tell CodeWriter that a child can be hoisted out by passing a `separate` flag. In theory, you want this to be `true` as often as possible. But passing `true` when the child isn't actually separate can lead to incorrect formatting results. In practice, the basic rule of thumb is a child can usually be formatted separately if the parent piece knows that there is a newline before it, there will be a newline after it, and there are no constraints being applied to any of its children. Further, we mostly only need to care about this optimization for the small number of pieces that are commonly used and can contain an unbounded number of children. That's: - SequencePiece - ChainPiece - ListPiece - InfixPiece In order to format the children of those separately, their children need to actually be pieces, so this also turns SequenceElement and ListElement into Piece classes. Also added a handful of benchmarks to validate this optimization on those various kinds of pieces.
1 parent 090eb85 commit cd0f2ab

32 files changed

+2134
-97
lines changed

‎benchmark/case/block.expect

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// Test statements nested within blocks.
2+
{
3+
{
4+
someLongOuterExpression +
5+
anotherOperand;
6+
someLongOuterExpression +
7+
anotherOperand;
8+
someLongOuterExpression +
9+
anotherOperand;
10+
someLongOuterExpression +
11+
anotherOperand;
12+
someLongOuterExpression +
13+
anotherOperand;
14+
}
15+
16+
someLongOuterExpression +
17+
anotherOperand;
18+
19+
{
20+
someLongOuterExpression +
21+
anotherOperand;
22+
someLongOuterExpression +
23+
anotherOperand;
24+
someLongOuterExpression +
25+
anotherOperand;
26+
someLongOuterExpression +
27+
anotherOperand;
28+
someLongOuterExpression +
29+
anotherOperand;
30+
}
31+
32+
someLongOuterExpression +
33+
anotherOperand;
34+
35+
{
36+
someLongOuterExpression +
37+
anotherOperand;
38+
someLongOuterExpression +
39+
anotherOperand;
40+
someLongOuterExpression +
41+
anotherOperand;
42+
someLongOuterExpression +
43+
anotherOperand;
44+
someLongOuterExpression +
45+
anotherOperand;
46+
}
47+
48+
someLongOuterExpression +
49+
anotherOperand;
50+
51+
{
52+
someLongOuterExpression +
53+
anotherOperand;
54+
someLongOuterExpression +
55+
anotherOperand;
56+
someLongOuterExpression +
57+
anotherOperand;
58+
someLongOuterExpression +
59+
anotherOperand;
60+
someLongOuterExpression +
61+
anotherOperand;
62+
}
63+
64+
someLongOuterExpression +
65+
anotherOperand;
66+
67+
{
68+
someLongOuterExpression +
69+
anotherOperand;
70+
someLongOuterExpression +
71+
anotherOperand;
72+
someLongOuterExpression +
73+
anotherOperand;
74+
someLongOuterExpression +
75+
anotherOperand;
76+
someLongOuterExpression +
77+
anotherOperand;
78+
}
79+
}

‎benchmark/case/block.expect_short

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// Test statements nested within blocks.
2+
{
3+
{
4+
someLongOuterExpression +
5+
anotherOperand;
6+
someLongOuterExpression +
7+
anotherOperand;
8+
someLongOuterExpression +
9+
anotherOperand;
10+
someLongOuterExpression +
11+
anotherOperand;
12+
someLongOuterExpression +
13+
anotherOperand;
14+
}
15+
16+
someLongOuterExpression +
17+
anotherOperand;
18+
19+
{
20+
someLongOuterExpression +
21+
anotherOperand;
22+
someLongOuterExpression +
23+
anotherOperand;
24+
someLongOuterExpression +
25+
anotherOperand;
26+
someLongOuterExpression +
27+
anotherOperand;
28+
someLongOuterExpression +
29+
anotherOperand;
30+
}
31+
32+
someLongOuterExpression +
33+
anotherOperand;
34+
35+
{
36+
someLongOuterExpression +
37+
anotherOperand;
38+
someLongOuterExpression +
39+
anotherOperand;
40+
someLongOuterExpression +
41+
anotherOperand;
42+
someLongOuterExpression +
43+
anotherOperand;
44+
someLongOuterExpression +
45+
anotherOperand;
46+
}
47+
48+
someLongOuterExpression +
49+
anotherOperand;
50+
51+
{
52+
someLongOuterExpression +
53+
anotherOperand;
54+
someLongOuterExpression +
55+
anotherOperand;
56+
someLongOuterExpression +
57+
anotherOperand;
58+
someLongOuterExpression +
59+
anotherOperand;
60+
someLongOuterExpression +
61+
anotherOperand;
62+
}
63+
64+
someLongOuterExpression +
65+
anotherOperand;
66+
67+
{
68+
someLongOuterExpression +
69+
anotherOperand;
70+
someLongOuterExpression +
71+
anotherOperand;
72+
someLongOuterExpression +
73+
anotherOperand;
74+
someLongOuterExpression +
75+
anotherOperand;
76+
someLongOuterExpression +
77+
anotherOperand;
78+
}
79+
}

‎benchmark/case/block.stmt

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
40 columns |
2+
// Test statements nested within blocks.
3+
{
4+
{
5+
someLongOuterExpression + anotherOperand;
6+
someLongOuterExpression + anotherOperand;
7+
someLongOuterExpression + anotherOperand;
8+
someLongOuterExpression + anotherOperand;
9+
someLongOuterExpression + anotherOperand;
10+
}
11+
12+
someLongOuterExpression + anotherOperand;
13+
14+
{
15+
someLongOuterExpression + anotherOperand;
16+
someLongOuterExpression + anotherOperand;
17+
someLongOuterExpression + anotherOperand;
18+
someLongOuterExpression + anotherOperand;
19+
someLongOuterExpression + anotherOperand;
20+
}
21+
22+
someLongOuterExpression + anotherOperand;
23+
24+
{
25+
someLongOuterExpression + anotherOperand;
26+
someLongOuterExpression + anotherOperand;
27+
someLongOuterExpression + anotherOperand;
28+
someLongOuterExpression + anotherOperand;
29+
someLongOuterExpression + anotherOperand;
30+
}
31+
32+
someLongOuterExpression + anotherOperand;
33+
34+
{
35+
someLongOuterExpression + anotherOperand;
36+
someLongOuterExpression + anotherOperand;
37+
someLongOuterExpression + anotherOperand;
38+
someLongOuterExpression + anotherOperand;
39+
someLongOuterExpression + anotherOperand;
40+
}
41+
42+
someLongOuterExpression + anotherOperand;
43+
44+
{
45+
someLongOuterExpression + anotherOperand;
46+
someLongOuterExpression + anotherOperand;
47+
someLongOuterExpression + anotherOperand;
48+
someLongOuterExpression + anotherOperand;
49+
someLongOuterExpression + anotherOperand;
50+
}
51+
}

‎benchmark/case/chain.expect

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Test long split method chains.
2+
target
3+
.someMethod1(
4+
argument1,
5+
argument2,
6+
argument3,
7+
)
8+
.someMethod2(
9+
argument1,
10+
argument2,
11+
argument3,
12+
)
13+
.someMethod3(
14+
argument1,
15+
argument2,
16+
argument3,
17+
)
18+
.someMethod4(
19+
argument1,
20+
argument2,
21+
argument3,
22+
)
23+
.someMethod5(
24+
argument1,
25+
argument2,
26+
argument3,
27+
)
28+
.someMethod6(
29+
argument1,
30+
argument2,
31+
argument3,
32+
)
33+
.someMethod7(
34+
argument1,
35+
argument2,
36+
argument3,
37+
)
38+
.someMethod8(
39+
argument1,
40+
argument2,
41+
argument3,
42+
)
43+
.someMethod9(
44+
argument1,
45+
argument2,
46+
argument3,
47+
)
48+
.someMethod10(
49+
argument1,
50+
argument2,
51+
argument3,
52+
)
53+
.someMethod11(
54+
argument1,
55+
argument2,
56+
argument3,
57+
)
58+
.someMethod12(
59+
argument1,
60+
argument2,
61+
argument3,
62+
)
63+
.someMethod13(
64+
argument1,
65+
argument2,
66+
argument3,
67+
)
68+
.someMethod14(
69+
argument1,
70+
argument2,
71+
argument3,
72+
)
73+
.someMethod15(
74+
argument1,
75+
argument2,
76+
argument3,
77+
)
78+
.someMethod16(
79+
argument1,
80+
argument2,
81+
argument3,
82+
)
83+
.someMethod17(
84+
argument1,
85+
argument2,
86+
argument3,
87+
)
88+
.someMethod18(
89+
argument1,
90+
argument2,
91+
argument3,
92+
)
93+
.someMethod19(
94+
argument1,
95+
argument2,
96+
argument3,
97+
)
98+
.someMethod20(
99+
argument1,
100+
argument2,
101+
argument3,
102+
);

‎benchmark/case/chain.expect_short

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Test long split method chains.
2+
target
3+
.someMethod1(
4+
argument1, argument2, argument3)
5+
.someMethod2(
6+
argument1, argument2, argument3)
7+
.someMethod3(
8+
argument1, argument2, argument3)
9+
.someMethod4(
10+
argument1, argument2, argument3)
11+
.someMethod5(
12+
argument1, argument2, argument3)
13+
.someMethod6(
14+
argument1, argument2, argument3)
15+
.someMethod7(
16+
argument1, argument2, argument3)
17+
.someMethod8(
18+
argument1, argument2, argument3)
19+
.someMethod9(
20+
argument1, argument2, argument3)
21+
.someMethod10(
22+
argument1, argument2, argument3)
23+
.someMethod11(
24+
argument1, argument2, argument3)
25+
.someMethod12(
26+
argument1, argument2, argument3)
27+
.someMethod13(
28+
argument1, argument2, argument3)
29+
.someMethod14(
30+
argument1, argument2, argument3)
31+
.someMethod15(
32+
argument1, argument2, argument3)
33+
.someMethod16(
34+
argument1, argument2, argument3)
35+
.someMethod17(
36+
argument1, argument2, argument3)
37+
.someMethod18(
38+
argument1, argument2, argument3)
39+
.someMethod19(
40+
argument1, argument2, argument3)
41+
.someMethod20(argument1, argument2,
42+
argument3);

‎benchmark/case/chain.stmt

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
40 columns |
2+
// Test long split method chains.
3+
target
4+
.someMethod1(argument1, argument2, argument3)
5+
.someMethod2(argument1, argument2, argument3)
6+
.someMethod3(argument1, argument2, argument3)
7+
.someMethod4(argument1, argument2, argument3)
8+
.someMethod5(argument1, argument2, argument3)
9+
.someMethod6(argument1, argument2, argument3)
10+
.someMethod7(argument1, argument2, argument3)
11+
.someMethod8(argument1, argument2, argument3)
12+
.someMethod9(argument1, argument2, argument3)
13+
.someMethod10(argument1, argument2, argument3)
14+
.someMethod11(argument1, argument2, argument3)
15+
.someMethod12(argument1, argument2, argument3)
16+
.someMethod13(argument1, argument2, argument3)
17+
.someMethod14(argument1, argument2, argument3)
18+
.someMethod15(argument1, argument2, argument3)
19+
.someMethod16(argument1, argument2, argument3)
20+
.someMethod17(argument1, argument2, argument3)
21+
.someMethod18(argument1, argument2, argument3)
22+
.someMethod19(argument1, argument2, argument3)
23+
.someMethod20(argument1, argument2, argument3);

0 commit comments

Comments
 (0)