Skip to content

Commit ec676a7

Browse files
Fix severe perf regression of complex nested function calls (#478)
* Add benchmark suite * Add benchmark action * Simplify benchmark * Use simple heuristics to reduce expontential blowup
1 parent 97c4b53 commit ec676a7

File tree

7 files changed

+174
-3
lines changed

7 files changed

+174
-3
lines changed

.github/workflows/benchmark.yml

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
name: Benchmark
2+
on:
3+
push:
4+
branches:
5+
- master
6+
7+
permissions:
8+
contents: write
9+
deployments: write
10+
11+
jobs:
12+
benchmark:
13+
name: Run Benchmark
14+
runs-on: ubuntu-latest
15+
steps:
16+
- uses: actions/checkout@v2
17+
# - run: rustup toolchain update nightly && rustup default nightly
18+
- name: Run benchmark
19+
run: cargo bench | tee output.txt # +nightly
20+
21+
- name: Store benchmark result
22+
uses: benchmark-action/github-action-benchmark@v1
23+
with:
24+
name: Rust Benchmark
25+
tool: "cargo"
26+
output-file-path: output.txt
27+
github-token: ${{ secrets.GITHUB_TOKEN }}
28+
auto-push: true
29+
# Show alert with commit comment on detecting possible performance regression
30+
alert-threshold: "200%"
31+
comment-on-alert: true
32+
fail-on-alert: true
33+
alert-comment-cc-users: "@johnnymorganz"

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1717
- [**Luau**] Increased the shape size of the expression in a type assertion so that it will correctly hang if over width ([#466](https://github.com/JohnnyMorganz/StyLua/issues/466))
1818
- Fixed binary expression in a table field containing a comment being collapsed leading to malformed formatted ([#471](https://github.com/JohnnyMorganz/StyLua/issues/471))
1919
- Fixed end parentheses of a function call with a multiline comment internally being expanded onto a new line unnecessarily ([#473](https://github.com/JohnnyMorganz/StyLua/issues/473))
20+
- Fixed severe performance regression with complex nested function calls ([#477](https://github.com/JohnnyMorganz/StyLua/issues/477))
2021

2122
## [0.13.1] - 2022-04-11
2223
### Fixed

Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,7 @@ harness = false
6060
[[bench]]
6161
name = "nested_tables"
6262
harness = false
63+
64+
[[bench]]
65+
name = "docgen"
66+
harness = false

benches/docgen.lua

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
-- https://github.com/neovim/nvim-lspconfig/blob/master/scripts/docgen.lua
2+
-- https://github.com/neovim/nvim-lspconfig/blob/master/LICENSE.md
3+
local preamble_parts = make_parts {
4+
function()
5+
if docs.description and #docs.description > 0 then
6+
return docs.description
7+
end
8+
end,
9+
function()
10+
local package_json_name = util.path.join(tempdir, template_name .. '.package.json')
11+
if docs.package_json then
12+
if not util.path.is_file(package_json_name) then
13+
os.execute(string.format('curl -v -L -o %q %q', package_json_name, docs.package_json))
14+
end
15+
if not util.path.is_file(package_json_name) then
16+
print(string.format('Failed to download package.json for %q at %q', template_name, docs.package_json))
17+
os.exit(1)
18+
return
19+
end
20+
local data = fn.json_decode(readfile(package_json_name))
21+
-- The entire autogenerated section.
22+
return make_section(0, '\n', {
23+
-- The default settings section
24+
function()
25+
local default_settings = (data.contributes or {}).configuration
26+
if not default_settings.properties then
27+
return
28+
end
29+
-- The outer section.
30+
return make_section(0, '\n', {
31+
'This server accepts configuration via the `settings` key.',
32+
'<details><summary>Available settings:</summary>',
33+
'',
34+
-- The list of properties.
35+
make_section(
36+
0,
37+
'\n\n',
38+
sorted_map_table(default_settings.properties, function(k, v)
39+
local function tick(s)
40+
return string.format('`%s`', s)
41+
end
42+
local function bold(s)
43+
return string.format('**%s**', s)
44+
end
45+
46+
-- https://github.github.com/gfm/#backslash-escapes
47+
local function excape_markdown_punctuations(str)
48+
local pattern =
49+
'\\(\\*\\|\\.\\|?\\|!\\|"\\|#\\|\\$\\|%\\|\'\\|(\\|)\\|,\\|-\\|\\/\\|:\\|;\\|<\\|=\\|>\\|@\\|\\[\\|\\\\\\|\\]\\|\\^\\|_\\|`\\|{\\|\\\\|\\|}\\)'
50+
return fn.substitute(str, pattern, '\\\\\\0', 'g')
51+
end
52+
53+
-- local function pre(s) return string.format("<pre>%s</pre>", s) end
54+
-- local function code(s) return string.format("<code>%s</code>", s) end
55+
if not (type(v) == 'table') then
56+
return
57+
end
58+
return make_section(0, '\n', {
59+
'- ' .. make_section(0, ': ', {
60+
bold(tick(k)),
61+
function()
62+
if v.enum then
63+
return tick('enum ' .. inspect(v.enum))
64+
end
65+
if v.type then
66+
return tick(table.concat(tbl_flatten { v.type }, '|'))
67+
end
68+
end,
69+
}),
70+
'',
71+
make_section(2, '\n\n', {
72+
{ v.default and 'Default: ' .. tick(inspect(v.default, { newline = '', indent = '' })) },
73+
{ v.items and 'Array items: ' .. tick(inspect(v.items, { newline = '', indent = '' })) },
74+
{ excape_markdown_punctuations(v.description) },
75+
}),
76+
})
77+
end)
78+
),
79+
'',
80+
'</details>',
81+
})
82+
end,
83+
})
84+
end
85+
end,
86+
}

benches/docgen.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
use criterion::{black_box, criterion_group, criterion_main, Criterion};
2+
use stylua_lib::{format_code, Config, OutputVerification};
3+
4+
pub fn format_docgen(c: &mut Criterion) {
5+
c.bench_function("format docgen.lua", |b| {
6+
b.iter(|| {
7+
format_code(
8+
black_box(include_str!("./docgen.lua")),
9+
black_box(Config::default()),
10+
black_box(None),
11+
black_box(OutputVerification::None),
12+
)
13+
})
14+
});
15+
}
16+
17+
criterion_group! {
18+
name = benches;
19+
config = Criterion::default().sample_size(40);
20+
targets = format_docgen
21+
}
22+
criterion_main!(benches);

src/formatters/functions.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,9 +184,17 @@ fn function_args_multiline_heuristic(
184184

185185
// Format all the arguments on an infinite width, so that we can prepare them and check to see whether they
186186
// need expanding. We will ignore punctuation for now
187-
let first_iter_formatted_arguments = arguments
188-
.iter()
189-
.map(|argument| format_expression(ctx, argument, shape.with_infinite_width()));
187+
let first_iter_formatted_arguments = arguments.iter().map(|argument| {
188+
if shape.using_simple_heuristics() {
189+
argument.to_owned()
190+
} else {
191+
format_expression(
192+
ctx,
193+
argument,
194+
shape.with_simple_heuristics().with_infinite_width(),
195+
)
196+
}
197+
});
190198

191199
// Apply some heuristics to determine whether we should expand the function call
192200
let mut singleline_shape = shape + PAREN_LEN;

src/shape.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,9 @@ pub struct Shape {
102102
offset: usize,
103103
/// The maximum number of characters we want to fit on a line. This is inferred from the configuration
104104
column_width: usize,
105+
/// Whether we should use simple heuristic checking.
106+
/// This is enabled when we are calling within a heuristic itself, to reduce the exponential blowup
107+
simple_heuristics: bool,
105108
}
106109

107110
impl Shape {
@@ -111,6 +114,7 @@ impl Shape {
111114
indent: Indent::new(ctx),
112115
offset: 0,
113116
column_width: ctx.config().column_width,
117+
simple_heuristics: false,
114118
}
115119
}
116120

@@ -179,6 +183,19 @@ impl Shape {
179183
}
180184
}
181185

186+
/// Whether simple heuristics should be used when calculating formatting shape
187+
/// This is to reduce the expontential blowup of discarded test formatting
188+
pub fn using_simple_heuristics(&self) -> bool {
189+
self.simple_heuristics
190+
}
191+
192+
pub fn with_simple_heuristics(&self) -> Shape {
193+
Self {
194+
simple_heuristics: true,
195+
..*self
196+
}
197+
}
198+
182199
/// Resets the offset for the shape
183200
pub fn reset(&self) -> Shape {
184201
Self { offset: 0, ..*self }

0 commit comments

Comments
 (0)