Optimize Lint/UselessAssign

[nobodywasishere]

Doing some basic performance metrics running ameba over the Crystal stdlib parser (src/compiler/crystal/syntax/parser.cr), most rules take a fraction of a second to run, except Lint/UselessAssign, which takes almost a second. Disabling this rule results in a significant boost in ameba performance (on this file). Granted, this isn't a great benchmark, esp since not all rules are in their worst-case performance wise, I think it's worth looking into if this rule can be optimized.

perf stats

Lint/UselessAssign: 0.850537000
Style/RedundantSelf: 0.040757000
Lint/Formatting: 0.010855000
Lint/Syntax: 0.009076000
Documentation/DocumentationAdmonition: 0.005054000
Lint/UnusedArgument: 0.005005000
Lint/ShadowedArgument: 0.004641000
Lint/MissingBlockArgument: 0.004399000
Lint/ShadowingOuterLocalVar: 0.003688000
Lint/SharedVarInFiber: 0.003485000
Lint/UnusedBlockArgument: 0.003378000
Lint/BadDirective: 0.002125000
Naming/VariableNames: 0.001809000
Lint/PercentArrays: 0.001708000
Layout/TrailingWhitespace: 0.001362000
Style/PercentLiteralDelimiters: 0.001342000
Lint/DuplicateWhenCondition: 0.001042000
Style/MultilineCurlyBlock: 0.001036000
Style/HeredocIndent: 0.000568000
Lint/AmbiguousAssignment: 0.000523000
Layout/TrailingBlankLines: 0.000478000
Metrics/CyclomaticComplexity: 0.000340000
Lint/DebugCalls: 0.000322000
Lint/NotNilAfterNoBang: 0.000304000
Lint/UselessConditionInWhen: 0.000292000
Lint/HashDuplicatedKey: 0.000287000
Lint/NotNil: 0.000285000
Performance/MinMaxAfterMap: 0.000280000
Lint/EmptyLoop: 0.000279000
Lint/EmptyEnsure: 0.000276000
Lint/DebuggerStatement: 0.000276000
Style/IsAFilter: 0.000272000
Lint/SignalTrap: 0.000262000
Performance/ChainedCallWithNoBang: 0.000261000
Lint/EmptyExpression: 0.000253000
Lint/LiteralsComparison: 0.000242000
Performance/ExcessiveAllocations: 0.000241000
Naming/AsciiIdentifiers: 0.000239000
Performance/FirstLastAfterFilter: 0.000238000
Lint/UnreachableCode: 0.000234000
Performance/CompactAfterMap: 0.000229000
Lint/LiteralAssignmentsInExpressions: 0.000227000
Performance/MapInsteadOfBlock: 0.000222000
Lint/RandZero: 0.000221000
Naming/AccessorMethodName: 0.000219000
Lint/ShadowedException: 0.000216000
Performance/FlattenAfterMap: 0.000215000
Lint/RequireParentheses: 0.000215000
Lint/VoidOutsideLib: 0.000215000
Performance/SizeAfterFilter: 0.000215000
Style/HeredocEscape: 0.000209000
Lint/LiteralInCondition: 0.000208000
Lint/RedundantWithObject: 0.000207000
Lint/RedundantWithIndex: 0.000206000
Style/NegatedConditionsInUnless: 0.000204000
Lint/LiteralInInterpolation: 0.000198000
Lint/RedundantStringCoercion: 0.000198000
Style/ParenthesesAroundCondition: 0.000195000
Lint/TrailingRescueException: 0.000193000
Naming/MethodNames: 0.000190000
Performance/AnyAfterFilter: 0.000187000
Naming/BlockParameterName: 0.000185000
Naming/BinaryOperatorParameterName: 0.000184000
Performance/AnyInsteadOfEmpty: 0.000182000
Lint/UnusedClassVariableAccess: 0.000181000
Naming/PredicateName: 0.000177000
Style/RedundantBegin: 0.000177000
Style/VerboseBlock: 0.000176000
Style/IsANil: 0.000172000
Naming/ConstantNames: 0.000172000
Style/UnlessElse: 0.000171000
Style/WhileTrue: 0.000166000
Naming/QueryBoolMethods: 0.000165000
Naming/TypeNames: 0.000162000
Lint/UnusedComparison: 0.000160000
Naming/RescuedExceptionsVariableName: 0.000160000
Style/RedundantNext: 0.000152000
Style/RedundantReturn: 0.000150000
Lint/UnusedGenericOrUnion: 0.000138000
Lint/UnusedInstanceVariableAccess: 0.000126000
Lint/UnusedLiteral: 0.000113000
Lint/UnusedLocalVariableAccess: 0.000111000
Lint/UnusedPseudoMethodCall: 0.000110000
Lint/UnusedSelfAccess: 0.000106000
Lint/DuplicatedRequire: 0.000004000
Naming/Filename: 0.000002000
Lint/Typos: 0.000002000
Lint/SpecFocus: 0.000001000
Lint/SpecFilename: 0.000001000

Benchmark diff

diff --git a/src/ameba/runner.cr b/src/ameba/runner.cr
index ebca6278..ed0c9d2a 100644
--- a/src/ameba/runner.cr
+++ b/src/ameba/runner.cr
@@ -138,12 +138,16 @@ module Ameba
         # We have to reprocess the source to pick up any changes. Since a
         # change could (theoretically) introduce syntax errors, we break the
         # loop if we find any.
+        start = Time.utc
         @syntax_rule.test(source)
+        puts "#{@syntax_rule.class.rule_name}: #{Time.utc - start}"
         break unless source.valid?
     @rules.each do |rule|
       next if rule.excluded?(source)



     start = Time.utc
     rule.test(source)



     puts "#{rule.class.rule_name}: #{Time.utc - start}"
   end
   check_unneeded_directives(source)
   break unless autocorrect? && source.correct?

[nobodywasishere]

Parser file:

$ time ameba --silent ../crystal/src/compiler/crystal/syntax/parser.cr
ameba --silent ../crystal/src/compiler/crystal/syntax/parser.cr  1.01s user 0.02s system 99% cpu 1.041 total

$ time ameba --silent ../crystal/src/compiler/crystal/syntax/parser.cr --except "Lint/UselessAssign"
ameba --silent ../crystal/src/compiler/crystal/syntax/parser.cr --except   0.09s user 0.03s system 128% cpu 0.095 total

Crystal compiler + stdlib:

$ time ameba --silent
ameba --silent  15.85s user 1.26s system 198% cpu 8.640 total

$ time ameba --silent --except "Lint/UselessAssign"
ameba --silent --except "Lint/UselessAssign"  8.32s user 1.14s system 154% cpu 6.117 total

[nobodywasishere]

Interestingly, looking at perf across the entire Crystal stdlib + compiler, it's possible Lint/UselessAssign has a worst-case just for that file. These are the summed times for all rules running on crystal-lang/crystal (keep in mind these times are across all threads, not concurrent):

perf stats

Lint/Syntax: 00:32:15.662589999
Documentation/DocumentationAdmonition: 00:14:44.794723999
Layout/TrailingBlankLines: 00:09:51.449565999
Layout/TrailingWhitespace: 00:09:21.061738999
Lint/BadDirective: 00:07:59.589259999
Lint/AmbiguousAssignment: 00:07:36.072944999
Lint/DebugCalls: 00:05:17.596066999
Lint/Formatting: 00:05:10.147332000
Lint/DebuggerStatement: 00:04:43.335006999
Lint/HashDuplicatedKey: 00:04:15.465278999
Lint/DuplicateWhenCondition: 00:03:56.950461999
Lint/EmptyEnsure: 00:03:34.110550999
Lint/EmptyExpression: 00:03:31.031790999
Lint/MissingBlockArgument: 00:03:21.427296999
Lint/DuplicatedRequire: 00:03:14.591677999
Lint/LiteralAssignmentsInExpressions: 00:03:02.956938999
Lint/PercentArrays: 00:02:48.791940000
Lint/NotNilAfterNoBang: 00:02:38.408929999
Lint/LiteralInInterpolation: 00:02:33.565171999
Lint/LiteralInCondition: 00:02:32.795049999
Lint/LiteralsComparison: 00:02:30.149725999
Lint/ShadowedException: 00:02:28.872042999
Lint/RandZero: 00:02:26.211870999
Lint/EmptyLoop: 00:02:25.603885999
Lint/ShadowingOuterLocalVar: 00:02:24.431907999
Lint/NotNil: 00:02:08.426207999
Lint/RedundantStringCoercion: 00:02:02.496783999
Lint/ShadowedArgument: 00:02:00.947028999
Lint/UnusedArgument: 00:01:43.970649000
Lint/UselessAssign: 00:01:42.364925999
Lint/SharedVarInFiber: 00:01:40.700544999
Lint/SignalTrap: 00:01:39.164701999
Lint/RedundantWithIndex: 00:01:25.760190999
Lint/RequireParentheses: 00:01:23.163343999
Lint/UnusedClassVariableAccess: 00:01:20.412118999
Lint/RedundantWithObject: 00:01:17.966706999
Lint/UnusedComparison: 00:01:04.670214999
Lint/UnreachableCode: 00:01:03.441339999
Lint/SpecFilename: 00:01:02.165644999
Lint/UselessConditionInWhen: 00:01:02.034945999
Lint/UnusedBlockArgument: 00:00:59.123692999
Lint/TopLevelOperatorDefinition: 00:00:59.111148999
Lint/VoidOutsideLib: 00:00:58.640335999
Metrics/CyclomaticComplexity: 00:00:50.989995999
Naming/BinaryOperatorParameterName: 00:00:50.379666999
Lint/SpecEqWithBoolOrNilLiteral: 00:00:46.907475999
Lint/UnusedLiteral: 00:00:46.577668999
Lint/TrailingRescueException: 00:00:46.565116999
Lint/UnusedInstanceVariableAccess: 00:00:46.519663999
Lint/UnusedGenericOrUnion: 00:00:44.923104999
Performance/FlattenAfterMap: 00:00:41.893219999
Naming/RescuedExceptionsVariableName: 00:00:41.507975999
Lint/UnusedSelfAccess: 00:00:40.728187000
Lint/UnusedLocalVariableAccess: 00:00:40.539492999
Naming/AsciiIdentifiers: 00:00:39.961458999
Lint/SpecFocus: 00:00:39.802350000
Naming/QueryBoolMethods: 00:00:38.727614000
Naming/BlockParameterName: 00:00:37.894666999
Naming/Filename: 00:00:36.826963000
Lint/UnusedPseudoMethodCall: 00:00:32.931687999
Style/IsAFilter: 00:00:31.357944000
Style/RedundantReturn: 00:00:31.292229000
Naming/ConstantNames: 00:00:30.319299000
Naming/AccessorMethodName: 00:00:30.265505000
Performance/AnyInsteadOfEmpty: 00:00:30.244119999
Lint/Typos: 00:00:29.500929000
Style/RedundantNext: 00:00:28.482277000
Style/PercentLiteralDelimiters: 00:00:26.638197000
Performance/ExcessiveAllocations: 00:00:26.352084000
Style/RedundantSelf: 00:00:26.143741000
Naming/TypeNames: 00:00:26.086025000
Naming/VariableNames: 00:00:25.932815000
Performance/FirstLastAfterFilter: 00:00:25.363793000
Naming/MethodNames: 00:00:25.101824000
Naming/PredicateName: 00:00:24.649761000
Style/WhileTrue: 00:00:24.482373000
Performance/MapInsteadOfBlock: 00:00:24.365368999
Performance/CompactAfterMap: 00:00:24.133545999
Performance/AnyAfterFilter: 00:00:23.156611999
Style/IsANil: 00:00:21.192257000
Performance/ChainedCallWithNoBang: 00:00:20.861990999
Style/NegatedConditionsInUnless: 00:00:20.766743000
Style/RedundantBegin: 00:00:18.819655999
Performance/SizeAfterFilter: 00:00:17.888476999
Performance/MinMaxAfterMap: 00:00:17.483893999
Style/ParenthesesAroundCondition: 00:00:17.454842000
Style/MultilineCurlyBlock: 00:00:16.372067999
Style/HeredocIndent: 00:00:16.247638999
Style/HeredocEscape: 00:00:12.493345999
Style/VerboseBlock: 00:00:12.365929999
Style/UnlessElse: 00:00:12.362010999

[nobodywasishere]

Same for ameba itself:

Lint/Syntax: 00:00:07.311781999
Documentation/DocumentationAdmonition: 00:00:06.175132999
Layout/TrailingBlankLines: 00:00:03.393369999
Lint/DebugCalls: 00:00:02.419910999
Layout/TrailingWhitespace: 00:00:02.276113999
Lint/BadDirective: 00:00:02.167798000
Lint/AmbiguousAssignment: 00:00:01.967264999
Lint/Formatting: 00:00:01.752459999
Lint/HashDuplicatedKey: 00:00:01.650494999
Lint/DebuggerStatement: 00:00:01.620956999
Lint/DuplicateWhenCondition: 00:00:01.575866999
Lint/DuplicatedRequire: 00:00:01.421888999
Lint/EmptyExpression: 00:00:01.333564999
Lint/LiteralsComparison: 00:00:01.271055999
Lint/NotNil: 00:00:01.245836999
Lint/LiteralAssignmentsInExpressions: 00:00:01.215458999
Lint/LiteralInCondition: 00:00:01.208501999
Lint/EmptyEnsure: 00:00:01.192506999
Lint/MissingBlockArgument: 00:00:01.174036999
Lint/RedundantStringCoercion: 00:00:01.140122999
Lint/EmptyLoop: 00:00:01.083422999
Lint/RandZero: 00:00:01.056099999
Lint/LiteralInInterpolation: 00:00:01.042764000
Lint/SignalTrap: 00:00:01.021868999
Lint/ShadowedArgument: 00:00:01.010762999
Lint/NotNilAfterNoBang: 0.942953000
Lint/ShadowedException: 0.922475000
Lint/UnusedArgument: 0.913925000
Lint/SpecEqWithBoolOrNilLiteral: 0.911592000
Lint/UnusedInstanceVariableAccess: 0.894534000
Lint/UnreachableCode: 0.864858000
Lint/RedundantWithObject: 0.859062000
Lint/Typos: 0.851825000
Lint/SpecFilename: 0.845902000
Lint/RequireParentheses: 0.827132000
Lint/ShadowingOuterLocalVar: 0.825677000
Lint/TrailingRescueException: 0.802033000
Lint/UnusedGenericOrUnion: 0.792442000
Lint/UnusedLocalVariableAccess: 0.769410000
Lint/UnusedClassVariableAccess: 0.761343000
Lint/UselessAssign: 0.753319000
Lint/TopLevelOperatorDefinition: 0.747966000
Lint/PercentArrays: 0.746378000
Lint/UnusedBlockArgument: 0.745340000
Lint/SpecFocus: 0.734317000
Naming/ConstantNames: 0.728575000
Lint/SharedVarInFiber: 0.723702000
Lint/UnusedPseudoMethodCall: 0.715216000
Lint/RedundantWithIndex: 0.708780000
Lint/UselessConditionInWhen: 0.656512000
Naming/VariableNames: 0.647247000
Lint/UnusedSelfAccess: 0.627718000
Naming/AccessorMethodName: 0.617353000
Naming/Filename: 0.607701000
Lint/UnusedLiteral: 0.583250000
Performance/FlattenAfterMap: 0.579644000
Lint/UnusedComparison: 0.566048000
Performance/AnyInsteadOfEmpty: 0.563684000
Performance/ExcessiveAllocations: 0.557843000
Performance/FirstLastAfterFilter: 0.531613000
Naming/BlockParameterName: 0.527857000
Lint/VoidOutsideLib: 0.526569000
Metrics/CyclomaticComplexity: 0.523478000
Performance/CompactAfterMap: 0.510906000
Performance/MapInsteadOfBlock: 0.510163000
Naming/TypeNames: 0.507030000
Style/IsAFilter: 0.493259000
Naming/AsciiIdentifiers: 0.492609000
Style/ParenthesesAroundCondition: 0.470493000
Naming/MethodNames: 0.469675000
Style/HeredocEscape: 0.467882000
Naming/QueryBoolMethods: 0.465625000
Performance/SizeAfterFilter: 0.444032000
Naming/BinaryOperatorParameterName: 0.439665000
Performance/ChainedCallWithNoBang: 0.438736000
Naming/RescuedExceptionsVariableName: 0.435913000
Naming/PredicateName: 0.434884000
Style/IsANil: 0.433183000
Style/MultilineCurlyBlock: 0.429236000
Style/UnlessElse: 0.422654000
Style/RedundantBegin: 0.402969000
Style/WhileTrue: 0.359865000
Performance/MinMaxAfterMap: 0.351246000
Style/NegatedConditionsInUnless: 0.350570000
Style/PercentLiteralDelimiters: 0.349566000
Style/RedundantNext: 0.332164000
Style/VerboseBlock: 0.309019000
Style/RedundantSelf: 0.308103000
Performance/AnyAfterFilter: 0.307158000
Style/HeredocIndent: 0.300697000
Style/RedundantReturn: 0.273428000

[straight-shoota]

parser.cr has some very long methods. That makes a large scope for local variables. Perhaps that's affecting performance there specifically?