[Performance Enhancements] Lambda expressions format change & Multi-level exploration logic changes

[crogoz]

Before this PR

Some repos would take 5 - 9 mins to be formatted. This is because we were exploring both preferBreakingLastInnerLevel & breakNormally for each level, with the branching coefficient capped to 20. The exploration is exponential. See below the Benchmark for the before implementation:

Benchmark                           Mode  Cnt  Score   Error  Units
BenchmarkMultiFiles.runNestedLevel  avgt   10  0.502 ± 0.009   s/op

Benchmark                                     Mode  Cnt  Score   Error  Units
BenchmarkMultiFiles.runNestedLevel           thrpt   10  1.985 ± 0.056  ops/s
BenchmarkMultiFiles.runNestedLevel            avgt   10  0.497 ± 0.007   s/op
BenchmarkMultiFiles.runNestedLevel          sample   45  0.499 ± 0.010   s/op
BenchmarkMultiFiles.runNestedLevel:p0.00    sample       0.484           s/op
BenchmarkMultiFiles.runNestedLevel:p0.50    sample       0.493           s/op
BenchmarkMultiFiles.runNestedLevel:p0.90    sample       0.520           s/op
BenchmarkMultiFiles.runNestedLevel:p0.95    sample       0.552           s/op
BenchmarkMultiFiles.runNestedLevel:p0.99    sample       0.583           s/op
BenchmarkMultiFiles.runNestedLevel:p0.999   sample       0.583           s/op
BenchmarkMultiFiles.runNestedLevel:p0.9999  sample       0.583           s/op
BenchmarkMultiFiles.runNestedLevel:p1.00    sample       0.583           s/op
BenchmarkMultiFiles.runNestedLevel              ss   10  0.513 ± 0.029   s/op

After this PR

Performance dashboards here. Internal excavator runs here

Example of the new diff: here or here

Most of the times we prefer preferBreakingLastInnerLevel in order to keep as compact as possible. I changed the logic that started the exploration top-down (with a branching coefficient capped at 20) to an exploration of the last LAST_LEVELS_TO_EXPLORE levels. For the last LAST_LEVELS_TO_EXPLORE I am exploring both preferBreakingLastInnerLevel & breakNormally and I will pick the formatted text that minimises the number of lines.
In order to keep the formatting as compact as possible, I am defaulting to preferBreakingLastInnerLevel for all the upper levels. If no such formatting was found, then I am falling back to breakNormally.

with LAST_LEVELS_TO_EXPLORE=8

Benchmark                                     Mode  Cnt   Score   Error  Units
BenchmarkMultiFiles.runNestedLevel           thrpt   10  26.514 ± 2.834  ops/s
BenchmarkMultiFiles.runNestedLevel            avgt   10   0.041 ± 0.004   s/op
BenchmarkMultiFiles.runNestedLevel          sample  518   0.039 ± 0.001   s/op
BenchmarkMultiFiles.runNestedLevel:p0.00    sample        0.032           s/op
BenchmarkMultiFiles.runNestedLevel:p0.50    sample        0.037           s/op
BenchmarkMultiFiles.runNestedLevel:p0.90    sample        0.043           s/op
BenchmarkMultiFiles.runNestedLevel:p0.95    sample        0.051           s/op
BenchmarkMultiFiles.runNestedLevel:p0.99    sample        0.088           s/op
BenchmarkMultiFiles.runNestedLevel:p0.999   sample        0.117           s/op
BenchmarkMultiFiles.runNestedLevel:p0.9999  sample        0.117           s/op
BenchmarkMultiFiles.runNestedLevel:p1.00    sample        0.117           s/op
BenchmarkMultiFiles.runNestedLevel              ss   10   0.042 ± 0.003   s/op

Because the number of exploration is substantially decreased, just changing the above setup would end up in some cases looking like here. I've noticed that most of the time the setup was wrong was for lambda expressions.

I changed the logic for lambda expressions (lambda body with no curly braces), to instead of trying to inline as much as possible the inner levels of the lambda body (code here) to defaulting to breakNormally:

• if the lambda expression fits completely on the same line as the parent, it will be inlined
• otherwise, the lambda body gets split in multiple lines.

---

Other things that I tried:

• with LAST_LEVELS_TO_EXPLORE = 12, noticed some perf regression for some repos.

Benchmark                                     Mode  Cnt  Score   Error  Units
BenchmarkMultiFiles.runNestedLevel           thrpt   10  5.427 ± 0.047  ops/s
BenchmarkMultiFiles.runNestedLevel            avgt   10  0.186 ± 0.005   s/op
BenchmarkMultiFiles.runNestedLevel          sample  110  0.188 ± 0.005   s/op
BenchmarkMultiFiles.runNestedLevel:p0.00    sample       0.180           s/op
BenchmarkMultiFiles.runNestedLevel:p0.50    sample       0.185           s/op
BenchmarkMultiFiles.runNestedLevel:p0.90    sample       0.192           s/op
BenchmarkMultiFiles.runNestedLevel:p0.95    sample       0.206           s/op
BenchmarkMultiFiles.runNestedLevel:p0.99    sample       0.273           s/op
BenchmarkMultiFiles.runNestedLevel:p0.999   sample       0.273           s/op
BenchmarkMultiFiles.runNestedLevel:p0.9999  sample       0.273           s/op
BenchmarkMultiFiles.runNestedLevel:p1.00    sample       0.273           s/op
BenchmarkMultiFiles.runNestedLevel              ss   10  0.197 ± 0.041   s/op

==COMMIT_MSG==
Improve perf by inspecting both breaking & not-breaking for the LAST_LEVELS_TO_EXPLORE levels
==COMMIT_MSG==

• with LAST_LEVELS_TO_EXPLORE = 7, however the formatting would regress in some cases (internal link here). Setting level = 12 fixed the issue.

Benchmark                                     Mode  Cnt   Score   Error  Units
BenchmarkMultiFiles.runNestedLevel           thrpt   10  35.640 ± 2.585  ops/s
BenchmarkMultiFiles.runNestedLevel            avgt   10   0.029 ± 0.002   s/op
BenchmarkMultiFiles.runNestedLevel          sample  705   0.029 ± 0.001   s/op
BenchmarkMultiFiles.runNestedLevel:p0.00    sample        0.024           s/op
BenchmarkMultiFiles.runNestedLevel:p0.50    sample        0.028           s/op
BenchmarkMultiFiles.runNestedLevel:p0.90    sample        0.032           s/op
BenchmarkMultiFiles.runNestedLevel:p0.95    sample        0.035           s/op
BenchmarkMultiFiles.runNestedLevel:p0.99    sample        0.046           s/op
BenchmarkMultiFiles.runNestedLevel:p0.999   sample        0.089           s/op
BenchmarkMultiFiles.runNestedLevel:p0.9999  sample        0.089           s/op
BenchmarkMultiFiles.runNestedLevel:p1.00    sample        0.089           s/op
BenchmarkMultiFiles.runNestedLevel              ss   10   0.029 ± 0.004   s/op

• Swapping the usage of fj.Set to HashSet. Pjf is trying to compute the line breaks and inserting into a fj.data.Set balancing the sorted set via comparisons. I tried swapping the fj.Set to using a normal HashSet, however this ended up being (more than 10x) slower. This is because the State is immutable and every time we create a new State, we need to copy & edit each Set field, which is much more optimal to do with the fj library than using a normal HashSet.
  See image below from the jfr.

Possible downsides?

[changelog-app]

Generate changelog in changelog/@unreleased

Type (Select exactly one)

• Feature (Adding new functionality)
• Improvement (Improving existing functionality)
• Fix (Fixing an issue with existing functionality)
• Break (Creating a new major version by breaking public APIs)
• Deprecation (Removing functionality in a non-breaking way)
• Migration (Automatically moving data/functionality to a new system)

Description

[Performance Enhancements] Lambda expressions format change & Multi-level exploration logic changes

Check the box to generate changelog(s)

• Generate changelog entry

[CRogers]

Approved for RC

Invalidated by push of b3de33c

[CRogers]

👍

[FinlayRJW]

👍

[CRogers]

👍

[kelvinou01]

for rc

Invalidated by push of de3b8eb

[changelog-app]

✅ Successfully generated changelog entry!

Need to regenerate?

Simply interact with the changelog bot comment again to regenerate these entries.

---

📋Changelog Preview

💡 Improvements

• [Performance Enhancements] Lambda expressions format change & Multi-level exploration logic changes (#1535)

[CRogers]

👍

[autorelease3]

Released 2.89.0

[v-chernyshev]

Good evening, please let me know if it's better to open an issue.

I would like to ask whether the following type of formatting changes introduced in 2.89.0 is fully intentional:

-        testing.getSuites().named(INTEGRATION_TEST_SUITE_NAME, JvmTestSuite.class, suite -> MainSourceSet.of(project)
-                .addAsCompileOnlyApiDependency(suite));
+        testing.getSuites()
+                .named(
+                        INTEGRATION_TEST_SUITE_NAME,
+                        JvmTestSuite.class,
+                        suite -> MainSourceSet.of(project).addAsCompileOnlyApiDependency(suite));

The old way was quite peculiar, but the new one is too eager to introduce line breaks and extra nesting, even when the lines are relatively short. Extracting code into temporary variables seems to be the only way to avoid the extra nesting, e.g.:

+        final var testingSuites = testing.getSuites();
+
-        testing.getSuites().named(INTEGRATION_TEST_SUITE_NAME, JvmTestSuite.class, suite -> MainSourceSet.of(project)
-                .addAsCompileOnlyApiDependency(suite));
+        testingSuites.named(
+                INTEGRATION_TEST_SUITE_NAME,
+                JvmTestSuite.class,
+                suite -> MainSourceSet.of(project).addAsCompileOnlyApiDependency(suite));