Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Severe performance regression with complex nested function calls #477

Closed
JohnnyMorganz opened this issue Jun 26, 2022 · 6 comments · Fixed by #478
Closed

Severe performance regression with complex nested function calls #477

JohnnyMorganz opened this issue Jun 26, 2022 · 6 comments · Fixed by #478
Labels
bug Something isn't working

Comments

@JohnnyMorganz
Copy link
Owner

File https://github.com/neovim/nvim-lspconfig/blob/master/scripts/docgen.lua takes 15s to format using latest master, and 133s (> 2min) on #476 branch!

With v13.1 it takes ~14s, and with v12 it takes 7s.

This is a large performance impact. This file should not be taking that long

@JohnnyMorganz JohnnyMorganz added the bug Something isn't working label Jun 26, 2022
@JohnnyMorganz

This comment was marked as outdated.

@JohnnyMorganz

This comment was marked as resolved.

@JohnnyMorganz
Copy link
Owner Author

This seems to have been present in many versions prior.
The main culprit seems to be this portion of code:

local preamble_parts = make_parts {
	function()
	  if docs.description and #docs.description > 0 then
		return docs.description
	  end
	end,
	function()
	  local package_json_name = util.path.join(tempdir, template_name .. '.package.json')
	  if docs.package_json then
		if not util.path.is_file(package_json_name) then
		  os.execute(string.format('curl -v -L -o %q %q', package_json_name, docs.package_json))
		end
		if not util.path.is_file(package_json_name) then
		  print(string.format('Failed to download package.json for %q at %q', template_name, docs.package_json))
		  os.exit(1)
		  return
		end
		local data = fn.json_decode(readfile(package_json_name))
		-- The entire autogenerated section.
		return make_section(0, '\n', {
		  -- The default settings section
		  function()
			local default_settings = (data.contributes or {}).configuration
			if not default_settings.properties then
			  return
			end
			-- The outer section.
			return make_section(0, '\n', {
			  'This server accepts configuration via the `settings` key.',
			  '<details><summary>Available settings:</summary>',
			  '',
			  -- The list of properties.
			  make_section(
				0,
				'\n\n',
				sorted_map_table(default_settings.properties, function(k, v)
				  local function tick(s)
					return string.format('`%s`', s)
				  end
				  local function bold(s)
					return string.format('**%s**', s)
				  end

				  -- https://github.github.com/gfm/#backslash-escapes
				  local function excape_markdown_punctuations(str)
					local pattern =
					  '\\(\\*\\|\\.\\|?\\|!\\|"\\|#\\|\\$\\|%\\|\'\\|(\\|)\\|,\\|-\\|\\/\\|:\\|;\\|<\\|=\\|>\\|@\\|\\[\\|\\\\\\|\\]\\|\\^\\|_\\|`\\|{\\|\\\\|\\|}\\)'
					return fn.substitute(str, pattern, '\\\\\\0', 'g')
				  end

				  -- local function pre(s) return string.format("<pre>%s</pre>", s) end
				  -- local function code(s) return string.format("<code>%s</code>", s) end
				  if not (type(v) == 'table') then
					return
				  end
				  return make_section(0, '\n', {
					'- ' .. make_section(0, ': ', {
					  bold(tick(k)),
					  function()
						if v.enum then
						  return tick('enum ' .. inspect(v.enum))
						end
						if v.type then
						  return tick(table.concat(tbl_flatten { v.type }, '|'))
						end
					  end,
					}),
					'',
					make_section(2, '\n\n', {
					  { v.default and 'Default: ' .. tick(inspect(v.default, { newline = '', indent = '' })) },
					  { v.items and 'Array items: ' .. tick(inspect(v.items, { newline = '', indent = '' })) },
					  { excape_markdown_punctuations(v.description) },
					}),
				  })
				end)
			  ),
			  '',
			  '</details>',
			})
		  end,
		})
	  end
	end,
  }

@JohnnyMorganz
Copy link
Owner Author

Removing unrelated code, this takes a long time:

local preamble_parts = make_parts {
	function()
	  do
		return make_section(0, '\n', {
		  -- The default settings section
		  function()
			return make_section(0, '\n', {
			  make_section(
				sorted_map_table(default_settings.properties, function(k, v)
				  return make_section(0, '\n', {
					'- ' .. make_section(0, ': ', {
					  bold(tick(k)),
					  function()
						if v.enum then
						  return tick('enum ' .. inspect(v.enum))
						end
						if v.type then
						  return tick(table.concat(tbl_flatten { v.type }, '|'))
						end
					  end,
					}),
					'',
					-- make_section(2, '\n\n', {
					--   { v.default and 'Default: ' .. tick(inspect(v.default, { newline = '', indent = '' })) },
					--   { v.items and 'Array items: ' .. tick(inspect(v.items, { newline = '', indent = '' })) },
					--   { excape_markdown_punctuations(v.description) },
					-- }),
				  })
				end)
			  ),
			})
		  end,
		})
	  end
	end,
  }

It seems though, if we remove the preliminary arguments to the function calls, like (0, '\n', , it speeds up - may be relevant

@JohnnyMorganz
Copy link
Owner Author

JohnnyMorganz commented Jun 26, 2022

Narrowed it down to the expensive function: function_args_multiline_heuristic

In particular, the call:

// Format all the arguments on an infinite width, so that we can prepare them and check to see whether they
// need expanding. We will ignore punctuation for now
let first_iter_formatted_arguments = arguments
    .iter()
    .map(|argument| format_expression(ctx, argument, shape.with_infinite_width()));

@JohnnyMorganz JohnnyMorganz changed the title Large performance regression Severe performance regression with complex nested function calls Jun 26, 2022
@JohnnyMorganz
Copy link
Owner Author

Some zombie-strike issues:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant