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

Limit number of structs in iex autocomplete #12674

Conversation

v0idpwn
Copy link
Contributor

@v0idpwn v0idpwn commented Jun 15, 2023

Getting structs for iex autocomplete is expensive: modules need to be loaded and checked to see if they define structs. This means that autocompleting for an empty struct name or a short one can be very slow in codebases with many modules and dependencies. In my work codebase, it might take several seconds for the autocomplete to return something and iex get stuck meanwhile.

This PR limits the number of structs, so that if a query that would return too many results is made, iex returns the completion quickly enough.

Sample:

v0idpwn ~/oss/elixir [feat/limit-number-of-structs-in-autocomplete] $ ./bin/iex
Erlang/OTP 25 [erts-13.1.4] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit]

Interactive Elixir (1.16.0-dev) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> %<TAB>
Config.Provider            Date
Date.Range                 DateTime
DynamicSupervisor          File.Stat
File.Stream                GenEvent.Stream
HashDict                   HashSet
IEx.History                IEx.Server
IO.Stream                  Inspect.Opts
Logger.Backends.Console    Logger.Formatter
Macro.Env                  MapSet
NaiveDateTime              Range
•••

Getting structs for iex autocomplete is expensive: modules need
to be loaded and checked to see if they define structs. This means
that autocompleting for an empty struct name or a short one can be
very slow in codebases with many modules and dependencies. In my
work codebase, it might take several seconds for the autocomplete
to return something and iex get stuck meanwhile.

This PR limits the number of structs, so that if a query that would
return too many results is made, iex returns the completion quickly
enough.
@v0idpwn
Copy link
Contributor Author

v0idpwn commented Jun 15, 2023

Will add tests, opening for early denial if you disagree with the solution. The limit is also up to discussion.

@josevalim
Copy link
Member

I wonder if we could try to optimize this first. We could use Code.ensure_all_loaded for example to load modules (which will be done in parallel IIRC).

@v0idpwn
Copy link
Contributor Author

v0idpwn commented Jun 15, 2023

Made a branch where I load the modules in advance, leveraging concurrency/parallelism: v0idpwn@276f2f8. No tests broken, everything behaves the same. Didn't bench, will need to setup a project for benching, probably will do it in the weekend.

@josevalim
Copy link
Member

Ping. :)

@josevalim
Copy link
Member

I pushed a commit that uses Code.ensure_all_loaded. I think with the code path cache from Erlang/OTP 26 and Elixir v1.15 with the Code.ensure_all_loaded, this is probably quite fast now :)

@v0idpwn
Copy link
Contributor Author

v0idpwn commented Aug 27, 2023

Thanks, @josevalim. I didn't manage to prioritise benching the solutions.

@v0idpwn v0idpwn deleted the feat/limit-number-of-structs-in-autocomplete branch August 27, 2023 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants