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

Design.AliasUsage let's the credo time sky rocket from ~11 seconds to ~127 seconds #877

Closed
PragTob opened this issue Jun 29, 2021 · 3 comments

Comments

@PragTob
Copy link
Contributor

PragTob commented Jun 29, 2021

First of all, hello there and thank you for your outstanding work! You as in @rrrene but also you as in every contributor ever you're amazing and have helped elixir devs all around the world so much! 🎉 🚀 💚

Also github tells me I last contributed in September 2016. Shame on me ;)

Environment

  • Credo version (mix credo -v): 1.5.6-ref.master.56a77e154+uncommittedchanges
  • Erlang/Elixir version (elixir -v):
Erlang/OTP 24 [erts-12.0.2] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit]

Elixir 1.12.1 (compiled with Erlang/OTP 24)
  • Operating system: OSX 11.3.1 ( :'( )

What were you trying to do?

Activate the AliasUsage lint on our code base with the following configuration:

{Credo.Check.Design.AliasUsage,
         [priority: :low, if_nested_deeper_than: 3, if_called_more_often_than: 1]}

Expected outcome

It increases the time to analysis the whole code base slightly.

Actual outcome

It 12xs the the code analysis time on the code base.

➜  ✗ mix credo --strict --ignore Credo.Check.Design.AliasUsage
Checking 1544 source files (this might take a while) ...

Please report incorrect results: https://github.com/rrrene/credo/issues

Analysis took 11.0 seconds (0.6s to load, 10.4s running 53 checks on 1544 files)
7666 mods/funs, found no issues.

Use `mix credo explain` to explain issues, `mix credo --help` for options.
➜  ✗ mix credo --strict
Generated credo app
Checking 1544 source files (this might take a while) ...

Please report incorrect results: https://github.com/rrrene/credo/issues

Analysis took 127.7 seconds (0.8s to load, 126.8s running 54 checks on 1544 files)
7666 mods/funs, found no issues.

Use `mix credo explain` to explain issues, `mix credo --help` for options.

Deactivating the lint of course solves the problem.

I don't think our configuration has a lot of impact on it, I also tried running it with the "default generated":

{Credo.Check.Design.AliasUsage,
         [priority: :low, if_nested_deeper_than: 2, if_called_more_often_than: 0]}

and it ended up at 96 seconds & 103 seconds. So, it might be slightly faster but still there is a clear increase.

For fun I tried:

{Credo.Check.Design.AliasUsage,
         [priority: :low, if_nested_deeper_than: 3, if_called_more_often_than: 4]},

(suspecting if_called_more_often_than to have an impact on time) and well it took 90 seconds. Ran it again with original config and it was 112 seconds. Dunno if this is just normal variance or if there's something to these that impacts it

😱


Anyhow, thanks a lot for everything 💚

IMG_20180617_160539

@rrrene
Copy link
Owner

rrrene commented Jul 4, 2021

Thank your for this bug report.

A cursory analysis yielded no obvious performance flaws in the check itself. Maybe it is just a slow check (we have to collect all references to all other modules from every module, after all).

I'll investigate further (any help on this would be appreaciated 👍).

@PragTob
Copy link
Contributor Author

PragTob commented Jul 5, 2021

👋

@rrrene anything specific you have in mind as help that I could provide other than some further tries investigation?

I guess best would be to have an OSS repo to reproduce but I can't OSS our code, maybe I could check if it happens on other big-ish elixir projects.

@rrrene
Copy link
Owner

rrrene commented Feb 3, 2023

I apologize for the age/inactivity on this issue. I should have done a better job at resolving this properly. 😥

Please feel free to re-open this issue at your discretion, especially if this is still a problem in your builds.

@rrrene rrrene closed this as completed Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants