From 07e72b7bf982f6bc84bd7728e613650505005fca Mon Sep 17 00:00:00 2001 From: Vitalii Elenhaupt <3624712+veelenga@users.noreply.github.com> Date: Sat, 9 Nov 2019 19:31:41 +0200 Subject: [PATCH] Lint in parallel (#118) * Lint in parallel * Synced output for dot/flycheck formatters * Re-raise exceptions raised in fibers * Add readme instructions --- README.md | 36 ++++++++++++++++++++ spec/ameba/runner_spec.cr | 13 ++++++++ spec/spec_helper.cr | 9 +++++ src/ameba/formatter/dot_formatter.cr | 13 ++++---- src/ameba/formatter/flycheck_formatter.cr | 10 ++++-- src/ameba/runner.cr | 40 ++++++++++++++++------- 6 files changed, 101 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 1fb4889f..800545bd 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,22 @@

+- [About](#about) +- [Usage](#usage) + * [Run in parallel](#run-in-parallel) +- [Installation](#installation) + * [As a project dependency:](#as-a-project-dependency) + * [OS X](#os-x) + * [Docker](#docker) + * [From sources](#from-sources) +- [Configuration](#configuration) + * [Only/Except](#onlyexcept) + * [Explanation](#explanation) + * [Inline disabling](#inline-disabling) +- [Editor integration](#editor-integration) +- [Credits & inspirations](#credits--inspirations) +- [Contributors](#contributors) + ## About Ameba is a static code analysis tool for the Crystal language. @@ -49,6 +65,26 @@ Finished in 542.64 milliseconds ``` +### Run in parallel + +Starting from 0.31.0 Crystal [supports parallelism](https://crystal-lang.org/2019/09/06/parallelism-in-crystal.html). +It allows to run linting in parallel too. +In order to take advantage of this feature you need to build ameba with preview_mt support: + +``` +$ crystal build src/cli.cr -Dpreview_mt -o bin/ameba +$ make install +``` + +Some quick benchmark results measured while running Ameba on Crystal repo: + +``` +$ CRYSTAL_WORKERS=1 ameba #=> 29.11 seconds +$ CRYSTAL_WORKERS=2 ameba #=> 19.49 seconds +$ CRYSTAL_WORKERS=4 ameba #=> 13.48 seconds +$ CRYSTAL_WORKERS=8 ameba #=> 10.14 seconds +``` + ## Installation ### As a project dependency: diff --git a/spec/ameba/runner_spec.cr b/spec/ameba/runner_spec.cr index 7dba438c..16957e68 100644 --- a/spec/ameba/runner_spec.cr +++ b/spec/ameba/runner_spec.cr @@ -53,6 +53,19 @@ module Ameba Runner.new(all_rules, [source], formatter, default_severity).run.success?.should be_true end + context "exception in rule" do + it "raises an exception raised in fiber while running a rule" do + rule = RaiseRule.new + rule.should_raise = true + rules = [rule] of Rule::Base + source = Source.new "", "source.cr" + + expect_raises(Exception, "something went wrong") do + Runner.new(rules, [source], formatter, default_severity).run + end + end + end + context "invalid syntax" do it "reports a syntax error" do rules = [Rule::Lint::Syntax.new] of Rule::Base diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index c08fb174..e9b6437e 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -81,6 +81,15 @@ module Ameba end end + # A rule that always raises an error + struct RaiseRule < Rule::Base + property should_raise = false + + def test(source) + should_raise && raise "something went wrong" + end + end + class DummyFormatter < Formatter::BaseFormatter property started_sources : Array(Source)? property finished_sources : Array(Source)? diff --git a/src/ameba/formatter/dot_formatter.cr b/src/ameba/formatter/dot_formatter.cr index 6cf51631..396069e5 100644 --- a/src/ameba/formatter/dot_formatter.cr +++ b/src/ameba/formatter/dot_formatter.cr @@ -7,6 +7,7 @@ module Ameba::Formatter include Util @started_at : Time? + @mutex = Thread::Mutex.new # Reports a message when inspection is started. def started(sources) @@ -18,12 +19,12 @@ module Ameba::Formatter # Reports a result of the inspection of a corresponding source. def source_finished(source : Source) sym = source.valid? ? ".".colorize(:green) : "F".colorize(:red) - output << sym - output.flush + @mutex.synchronize { output << sym } end # Reports a message when inspection is finished. def finished(sources) + output.flush output << "\n\n" show_affected_code = !config[:without_affected_code]? @@ -38,7 +39,7 @@ module Ameba::Formatter output << "[#{issue.rule.severity.symbol}] #{issue.rule.name}: #{issue.message}\n".colorize(:red) if show_affected_code && (code = affected_code(source, location)) - output << code + output << code.colorize(:default) end output << "\n" @@ -51,15 +52,15 @@ module Ameba::Formatter private def started_message(size) if size == 1 - "Inspecting 1 file.\n\n" + "Inspecting 1 file.\n\n".colorize(:default) else - "Inspecting #{size} files.\n\n" + "Inspecting #{size} files.\n\n".colorize(:default) end end private def finished_in_message(started, finished) if started && finished - "Finished in #{to_human(finished - started)} \n\n" + "Finished in #{to_human(finished - started)} \n\n".colorize(:default) end end diff --git a/src/ameba/formatter/flycheck_formatter.cr b/src/ameba/formatter/flycheck_formatter.cr index 181eb8be..89423ff3 100644 --- a/src/ameba/formatter/flycheck_formatter.cr +++ b/src/ameba/formatter/flycheck_formatter.cr @@ -1,12 +1,16 @@ module Ameba::Formatter class FlycheckFormatter < BaseFormatter + @mutex = Mutex.new + def source_finished(source : Source) source.issues.each do |e| next if e.disabled? if loc = e.location - output.printf "%s:%d:%d: %s: [%s] %s\n", - source.path, loc.line_number, loc.column_number, e.rule.severity.symbol, - e.rule.name, e.message.gsub("\n", " ") + @mutex.synchronize do + output.printf "%s:%d:%d: %s: [%s] %s\n", + source.path, loc.line_number, loc.column_number, e.rule.severity.symbol, + e.rule.name, e.message.gsub("\n", " ") + end end end end diff --git a/src/ameba/runner.cr b/src/ameba/runner.cr index 0677c666..ece39187 100644 --- a/src/ameba/runner.cr +++ b/src/ameba/runner.cr @@ -68,24 +68,42 @@ module Ameba # def run @formatter.started @sources - @sources.each do |source| - @formatter.source_started source - - if @syntax_rule.catch(source).valid? - @rules.each do |rule| - next if rule.excluded?(source) - rule.test(source) - end - check_unneeded_directives(source) + channels = @sources.map { Channel(Exception?).new } + @sources.each_with_index do |source, idx| + channel = channels[idx] + spawn do + run_source(source) + rescue e + channel.send(e) + else + channel.send(nil) end - - @formatter.source_finished source end + + channels.each do |c| + e = c.receive + raise e unless e.nil? + end + self ensure @formatter.finished @sources end + private def run_source(source) + @formatter.source_started source + + if @syntax_rule.catch(source).valid? + @rules.each do |rule| + next if rule.excluded?(source) + rule.test(source) + end + check_unneeded_directives(source) + end + + @formatter.source_finished source + end + # Explains an issue at a specified *location*. # # Runner should perform inspection before doing the explain.