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

The Style/MapCompactWithConditionalBlock cop must be executed before or instead of the Performance/MapCompact cop #414

Open
ydakuka opened this issue Dec 2, 2023 · 5 comments

Comments

@ydakuka
Copy link
Contributor

ydakuka commented Dec 2, 2023

See comments for more information.

@ydakuka ydakuka changed the title Cop Style/MapCompactWithConditionalBlock has similar behavior with the Performance/MapCompact cop [WIP] Cop Style/MapCompactWithConditionalBlock has similar behavior with the Performance/MapCompact cop Dec 2, 2023
@ydakuka ydakuka changed the title [WIP] Cop Style/MapCompactWithConditionalBlock has similar behavior with the Performance/MapCompact cop [WIP] The Style/MapCompactWithConditionalBlock cop has similar behavior with the Performance/MapCompact cop Dec 2, 2023
@ydakuka
Copy link
Contributor Author

ydakuka commented Dec 2, 2023

I have the Dockerfile:

FROM ruby:3.3.0
COPY . /var/www/ruby  
WORKDIR /var/www/ruby  
RUN gem install benchmark-ips
RUN ruby -v
CMD ["ruby", "file.rb"]

@ydakuka
Copy link
Contributor Author

ydakuka commented Dec 2, 2023

The code is from the first "bad" example:

1 case

The condition is false.

require 'benchmark/ips'

ARRAY = (1..100).to_a

def initial
  ARRAY.map { |e| false ? e : next }.compact
end

def slow
  ARRAY.filter_map { |e| false ? e : next }
end

def fast
  ARRAY.select { |e| false }
end

Benchmark.ips do |x|
  x.report('initial') { initial }
  x.report('Array#filter_map') { slow }
  x.report('Array#select') { fast }
  x.compare!
end

Output:

ydakuka@yauhenid:~/ruby-docker-app$ docker run ruby-app  
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-linux]
Warming up --------------------------------------
             initial    19.422k i/100ms
    Array#filter_map    16.256k i/100ms
        Array#select    24.173k i/100ms
Calculating -------------------------------------
             initial    182.889k (± 4.3%) i/s -    932.256k in   5.108206s
    Array#filter_map    173.925k (± 4.1%) i/s -    877.824k in   5.056780s
        Array#select    221.553k (± 6.3%) i/s -      1.112M in   5.047671s

Comparison:
        Array#select:   221553.1 i/s
             initial:   182888.7 i/s - 1.21x  slower
    Array#filter_map:   173924.8 i/s - 1.27x  slower

2 case

The condition is true.

require 'benchmark/ips'

ARRAY = (1..100).to_a

def initial
  ARRAY.map { |e| true ? e : next }.compact
end

def slow
  ARRAY.filter_map { |e| true ? e : next }
end

def fast
  ARRAY.select { |e| true }
end

Benchmark.ips do |x|
  x.report('initial') { initial }
  x.report('Array#filter_map') { slow }
  x.report('Array#select') { fast }
  x.compare!
end

Output:

ydakuka@yauhenid:~/ruby-docker-app$ docker run ruby-app  
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-linux]
Warming up --------------------------------------
             initial    18.930k i/100ms
    Array#filter_map    12.068k i/100ms
        Array#select    17.634k i/100ms
Calculating -------------------------------------
             initial    176.136k (± 3.9%) i/s -    889.710k in   5.059519s
    Array#filter_map    128.785k (± 3.5%) i/s -    651.672k in   5.066690s
        Array#select    181.992k (± 3.6%) i/s -    916.968k in   5.045294s

Comparison:
        Array#select:   181992.4 i/s
             initial:   176135.7 i/s - same-ish: difference falls within error
    Array#filter_map:   128784.7 i/s - 1.41x  slower

@ydakuka
Copy link
Contributor Author

ydakuka commented Dec 2, 2023

The code is from the second "bad" example:

1 case

The condition is false.

require 'benchmark/ips'

ARRAY = (1..100).to_a

def initial
  ARRAY.map do |e|
    if false
      e
    else
      next
    end
  end.compact
end

def slow
  ARRAY.filter_map do |e|
    if false
      e
    else
      next
    end
  end
end

def fast
  ARRAY.select { |e| false }
end

Benchmark.ips do |x|
  x.report('initial') { initial }
  x.report('Array#filter_map') { slow }
  x.report('Array#select') { fast }
  x.compare!
end

Output:

ydakuka@yauhenid:~/ruby-docker-app$ docker run ruby-app  
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-linux]
Warming up --------------------------------------
             initial    17.535k i/100ms
    Array#filter_map    18.274k i/100ms
        Array#select    23.798k i/100ms
Calculating -------------------------------------
             initial    165.717k (± 7.4%) i/s -    824.145k in   5.007599s
    Array#filter_map    154.578k (± 3.9%) i/s -    785.782k in   5.091466s
        Array#select    217.905k (± 2.2%) i/s -      1.095M in   5.026222s

Comparison:
        Array#select:   217904.5 i/s
             initial:   165716.6 i/s - 1.31x  slower
    Array#filter_map:   154578.1 i/s - 1.41x  slower

2 case

The condition is true.

require 'benchmark/ips'

ARRAY = (1..100).to_a

def initial
  ARRAY.map do |e|
    if true
      e
    else
      next
    end
  end.compact
end

def slow
  ARRAY.filter_map do |e|
    if true
      e
    else
      next
    end
  end
end

def fast
  ARRAY.select { |e| true }
end

Benchmark.ips do |x|
  x.report('initial') { initial }
  x.report('Array#filter_map') { slow }
  x.report('Array#select') { fast }
  x.compare!
end

Output:

ydakuka@yauhenid:~/ruby-docker-app$ docker run ruby-app  
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-linux]
Warming up --------------------------------------
             initial    17.664k i/100ms
    Array#filter_map    12.426k i/100ms
        Array#select    17.116k i/100ms
Calculating -------------------------------------
             initial    166.107k (± 5.1%) i/s -    830.208k in   5.012504s
    Array#filter_map    120.492k (± 4.6%) i/s -    608.874k in   5.064135s
        Array#select    183.231k (± 3.3%) i/s -    924.264k in   5.049806s

Comparison:
        Array#select:   183230.9 i/s
             initial:   166107.4 i/s - 1.10x  slower
    Array#filter_map:   120491.8 i/s - 1.52x  slower

@ydakuka
Copy link
Contributor Author

ydakuka commented Dec 2, 2023

The code is from the third "bad" example:

1 case

The condition is false.

require 'benchmark/ips'

ARRAY = (1..100).to_a

def initial
  ARRAY.map do |e|
    next if false

    e
  end.compact
end

def slow
  ARRAY.filter_map do |e|
    next if false

    e
  end
end

def fast
  ARRAY.select { |e| false }
end

Benchmark.ips do |x|
  x.report('initial') { initial }
  x.report('Array#filter_map') { slow }
  x.report('Array#select') { fast }
  x.compare!
end

Output:

ydakuka@yauhenid:~/ruby-docker-app$ docker run ruby-app  
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-linux]
Warming up --------------------------------------
             initial    17.851k i/100ms
    Array#filter_map    12.558k i/100ms
        Array#select    20.528k i/100ms
Calculating -------------------------------------
             initial    165.176k (± 7.3%) i/s -    838.997k in   5.108443s
    Array#filter_map    115.084k (± 4.0%) i/s -    577.668k in   5.028230s
        Array#select    202.981k (± 6.0%) i/s -      1.026M in   5.080126s

Comparison:
        Array#select:   202981.4 i/s
             initial:   165175.9 i/s - 1.23x  slower
    Array#filter_map:   115084.4 i/s - 1.76x  slower

2 case

The condition is true.

require 'benchmark/ips'

ARRAY = (1..100).to_a

def initial
  ARRAY.map do |e|
    next if true

    e
  end.compact
end

def slow
  ARRAY.filter_map do |e|
    next if true

    e
  end
end

def fast
  ARRAY.select { |e| true }
end

Benchmark.ips do |x|
  x.report('initial') { initial }
  x.report('Array#filter_map') { slow }
  x.report('Array#select') { fast }
  x.compare!
end

Output:

ydakuka@yauhenid:~/ruby-docker-app$ docker run ruby-app  
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-linux]
Warming up --------------------------------------
             initial    17.313k i/100ms
    Array#filter_map    17.244k i/100ms
        Array#select    18.456k i/100ms
Calculating -------------------------------------
             initial    170.691k (± 4.1%) i/s -    865.650k in   5.080156s
    Array#filter_map    157.454k (± 1.3%) i/s -    793.224k in   5.038639s
        Array#select    178.570k (± 3.4%) i/s -    904.344k in   5.071589s

Comparison:
        Array#select:   178569.8 i/s
             initial:   170691.3 i/s - same-ish: difference falls within error
    Array#filter_map:   157454.4 i/s - 1.13x  slower

@ydakuka
Copy link
Contributor Author

ydakuka commented Dec 2, 2023

The code is from the fourth "bad" example:

1 case

The condition is false.

require 'benchmark/ips'

ARRAY = (1..100).to_a

def initial
  ARRAY.map do |e|
    e if false
  end.compact
end

def slow
  ARRAY.filter_map do |e|
    e if false
  end
end

def fast
  ARRAY.select { |e| false }
end

Benchmark.ips do |x|
  x.report('initial') { initial }
  x.report('Array#filter_map') { slow }
  x.report('Array#select') { fast }
  x.compare!
end

Output:

ydakuka@yauhenid:~/ruby-docker-app$ docker run ruby-app  
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-linux]
Warming up --------------------------------------
             initial    16.860k i/100ms
    Array#filter_map    16.718k i/100ms
        Array#select    22.405k i/100ms
Calculating -------------------------------------
             initial    167.713k (± 5.1%) i/s -    843.000k in   5.041983s
    Array#filter_map    157.887k (± 3.6%) i/s -    802.464k in   5.089215s
        Array#select    212.752k (± 3.5%) i/s -      1.075M in   5.061251s

Comparison:
        Array#select:   212751.6 i/s
             initial:   167713.5 i/s - 1.27x  slower
    Array#filter_map:   157887.2 i/s - 1.35x  slower

2 case

The condition is true.

require 'benchmark/ips'

ARRAY = (1..100).to_a

def initial
  ARRAY.map do |e|
    e if true
  end.compact
end

def slow
  ARRAY.filter_map do |e|
    e if true
  end
end

def fast
  ARRAY.select { |e| true }
end

Benchmark.ips do |x|
  x.report('initial') { initial }
  x.report('Array#filter_map') { slow }
  x.report('Array#select') { fast }
  x.compare!
end

Output:

ydakuka@yauhenid:~/ruby-docker-app$ docker run ruby-app  
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-linux]
Warming up --------------------------------------
             initial    17.061k i/100ms
    Array#filter_map    12.748k i/100ms
        Array#select    18.512k i/100ms
Calculating -------------------------------------
             initial    173.894k (± 1.8%) i/s -    870.111k in   5.005424s
    Array#filter_map    118.572k (± 4.1%) i/s -    599.156k in   5.061249s
        Array#select    175.916k (± 3.9%) i/s -    888.576k in   5.058945s

Comparison:
        Array#select:   175916.3 i/s
             initial:   173894.5 i/s - same-ish: difference falls within error
    Array#filter_map:   118571.6 i/s - 1.48x  slower

@ydakuka ydakuka changed the title [WIP] The Style/MapCompactWithConditionalBlock cop has similar behavior with the Performance/MapCompact cop The Style/MapCompactWithConditionalBlock cop has similar behavior with the Performance/MapCompact cop Dec 2, 2023
@ydakuka ydakuka changed the title The Style/MapCompactWithConditionalBlock cop has similar behavior with the Performance/MapCompact cop The Style/MapCompactWithConditionalBlock cop must be executed before or instead of the Performance/MapCompact cop Dec 2, 2023
@ydakuka ydakuka changed the title The Style/MapCompactWithConditionalBlock cop must be executed before or instead of the Performance/MapCompact cop The Style/MapCompactWithConditionalBlock cop must be executed before or instead of the Performance/MapCompact cop Jan 12, 2024
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

1 participant