Slow credo runs since 1.15.x

[Eein]

Environment

• Credo version (mix credo -v): 1.7.0 & master
• Erlang/Elixir version (elixir -v): Elixir 1.15.3 OTP/26
• Operating system: Fedora Linux Kernel 6.3.10
• CPU - AMD 5900x (12c/24t)

What were you trying to do?

Run credo on a 5388 files

Expected outcome

Since updating to elixir 1.15.x, credo takes a much longer time to run. In the past, it was usually around 30-40 seconds on 1.14.

Actual outcome

Analysis took 217.0 seconds (0.7s to load, 216.2s running 54 checks on 5388 files)
23351 mods/funs, found 2 refactoring opportunities.

Additional Details

After debugging for a bit to try and find out where there may be a bottleneck, I started toying with: https://github.com/rrrene/credo/blob/master/lib/credo/execution.ex#L122

After changing max_concurrent_check_runs to 1, credo runs extremely fast;

Analysis took 12.0 seconds (0.7s to load, 11.2s running 54 checks on 5388 files)
23351 mods/funs, found 2 refactoring opportunities.

I've tried combinations of earlier OTP versions with verious path versions of 1.15 as well with the same results.

I'm curious if theres an issue with the worker/runner setup in 1.15.x when running on more than a single core.

Edit: After testing with work CI, here are my results:

---

Old

Analysis took 73.1 seconds (4.1s to load, 68.9s running 52 checks on 5969 files)
24446 mods/funs, found no issues.

---

After setting max_concurrent_check_runs to 1

Analysis took 63.3 seconds (4.1s to load, 59.2s running 55 checks on 5968 files)
24439 mods/funs, found 2 refactoring opportunities.

This may confirm my suspicion that something is up with the worker/runner setup when using more than 1 scheduler.

Thanks!

[rrrene]

Thanks for taking the time to report this and write up your findings. 👍

This is definitely something we have to look into!

[ulissesalmeida]

I think it might be related to OTP/26.

I was running with Elixir 1.15 OTP/25 on a large circle instance,
when I did the upgrade to OTP/26 I had to upgrade to xlarge instance, otherwise the job would be killed 'cause it was using too much memory.

[ulissesalmeida]

Sharing also timing data:

OTP 25
It was taking ~2 minutes, large instance to run

OTP 26
It takes ~6 minutes, xlarge instance to run

The circle ci instances:
https://circleci.com/product/features/resource-classes/

large: 4 CPU, 8 GB memory ram
xlarge: 8 CPU, 16 GB memory ram

So, with OTP 26, credo is running ~6 times slower and consuming more memory

[Eein]

I've been meaning to take a look at this in the next week or two - assuming nothing takes precedence I'll give it a stab and see if I can extract performance. Are the elixir version in the github actions the main targets for credo?

I'm just curious if I can potentially take advantage of dynamic supervisors to accomplish the goal.

[rrrene]

That would be great. We are targeting the last five minor releases of Elixir 👍

[Eein]

haven't been able to get back to this just yet - work/family - but i'm still interested in tackling this eventually. If someone jumps on this let me know :)

[Eein]

I've done a bit of testing, and changed check/worker.ex inner_loop to:

  defp inner_loop(worker_context, [workload | rest], taken) do
    task = Task.async(fn -> worker_context.work_fn.(workload) end)
    result = Task.await(task, :infinity)
    send_workload_finished_to_runner(worker_context, workload, result)
    inner_loop(worker_context, rest, taken + 1)
  end

replacing the existing spawn that was there and now we're around the same speed as a max_concurrency 1 (maybe a bit faster)

if you set max concurrency to 1, it uses all CPU's available, so i wonder if the optimization is actually a mistake and elixir is too efficient. :)

EDIT: I've removed the spawn/Task.async functions entirely and it is as quick as max concurrency and uses all cores by default

EDITEDIT: I've gone as far as completely removing the the worker, and its slightly slower than max_concurrency 1 or Task.async

defmodule Credo.Check.Runner do
  @moduledoc false

  # This module is responsible for running checks based on the context represented
  # by the current `Credo.Execution`.

  alias Credo.Check.Params
  alias Credo.CLI.Output.UI
  alias Credo.Execution
  alias Credo.Execution.ExecutionTiming

  @doc """
  Runs all checks on all source files (according to the config).
  """
  def run(source_files, exec) when is_list(source_files) do
    check_tuples =
      exec
      |> Execution.checks()
      |> warn_about_ineffective_patterns(exec)
      |> fix_deprecated_notation_for_checks_without_params()

    Enum.each(check_tuples, fn ct ->
      run_check(exec, ct)
    end)

    :ok
  end

After some looking:
It looks like this behaviour is already replicated in run_all_source_files which is whats causing the delay for the concurrency

      defp do_run_on_all_source_files(exec, source_files, params) do
        source_files
        |> Enum.map(&Task.async(fn -> run_on_source_file(exec, &1, params) end))
        |> Enum.each(&Task.await(&1, :infinity))

        :ok
      end

So at this point i'd assume that you'd be running all tests on every max_concurrency - causing the slowdown? :P