Enable cop Performance/OpenStruct

Description of the proposal

Ban all occurrences of OpenStruct in non-spec code by enabling Performance/OpenStruct.

This cop checks for OpenStruct.new calls. Instantiation of an OpenStruct invalidates Ruby global method cache (see ) as it causes dynamic method definition during program runtime. This could have an effect on performance, especially in case of single-threaded applications with multiple OpenStruct instantiations.

Using Struct.new(..., keyword_init: true) is much faster than OpenStruct.new(...) as it does not define methods during runtime.

Misspelled method names raise NoMethodError instead of silently returning nil.

This cop is not auto-correctable and not every offense can be replaced with Struct easily.

Current offenses

Example:

ee/lib/gitlab/graphql/aggregations/epics/epic_node.rb:44:26: C: Performance/OpenStruct: Consider using Struct over OpenStruct to optimize the performance.
              OpenStruct.new({
                         ^^^

gitlab-org/gitlab

Offenses:

Guardfile:12:14: C: Performance/OpenStruct: Consider using Struct over OpenStruct to optimize the performance.
  OpenStruct.new(to_s: "spec").tap do |rspec|
             ^^^
Guardfile:22:14: C: Performance/OpenStruct: Consider using Struct over OpenStruct to optimize the performance.
  OpenStruct.new.tap do |rails|
             ^^^
app/finders/snippets_finder.rb:50:26: C: Performance/OpenStruct: Consider using Struct over OpenStruct to optimize the performance.
    @params = OpenStruct.new(params)
                         ^^^
app/helpers/application_settings_helper.rb:40:74: C: Performance/OpenStruct: Consider using Struct over OpenStruct to optimize the performance.
    Gitlab.config.repositories.storages.keys.each_with_object(OpenStruct.new) do |storage, weights|
                                                                         ^^^
app/models/cycle_analytics/project_level_stage_adapter.rb:16:43: C: Performance/OpenStruct: Consider using Struct over OpenStruct to optimize the performance.
      serializer.new.represent(OpenStruct.new(
                                          ^^^
ee/lib/gitlab/graphql/aggregations/epics/epic_node.rb:33:26: C: Performance/OpenStruct: Consider using Struct over OpenStruct to optimize the performance.
              OpenStruct.new({
                         ^^^
ee/lib/gitlab/graphql/aggregations/epics/epic_node.rb:44:26: C: Performance/OpenStruct: Consider using Struct over OpenStruct to optimize the performance.
              OpenStruct.new({
                         ^^^
lib/api/wikis.rb:137:32: C: Performance/OpenStruct: Consider using Struct over OpenStruct to optimize the performance.
            present OpenStruct.new(result[:result]), with: Entities::WikiAttachment
                               ^^^
lib/gitlab/ci/ansi2html.rb:283:22: C: Performance/OpenStruct: Consider using Struct over OpenStruct to optimize the performance.
          OpenStruct.new(
                     ^^^
lib/gitlab/ci/reports/test_suite_comparer.rb:93:24: C: Performance/OpenStruct: Consider using Struct over OpenStruct to optimize the performance.
            OpenStruct.new(
                       ^^^
lib/gitlab/git/diff_collection.rb:34:20: C: Performance/OpenStruct: Consider using Struct over OpenStruct to optimize the performance.
        OpenStruct.new(limits)
                   ^^^
lib/gitlab/import_export/after_export_strategies/base_after_export_strategy.rb:20:33: C: Performance/OpenStruct: Consider using Struct over OpenStruct to optimize the performance.
          @options = OpenStruct.new(attributes)
                                ^^^
lib/gitlab/testing/request_inspector_middleware.rb:43:30: C: Performance/OpenStruct: Consider using Struct over OpenStruct to optimize the performance.
        request = OpenStruct.new(
                             ^^^
lib/mattermost/session.rb:67:31: C: Performance/OpenStruct: Consider using Struct over OpenStruct to optimize the performance.
      @request ||= OpenStruct.new(parameters: params)
                              ^^^

22247 files inspected, 14 offenses detected

Benchmarks

Script

require "benchmark/ips"
require "ostruct"

struct = Struct.new(:a, :b, :c, :d, :e, keyword_init: true)
args = { a: 1, b: 2, c: 3, d: 4, e: 5 }

Benchmark.ips do |x|
  x.report "openstruct new" do
    OpenStruct.new(args)
  end

  x.report "openstruct access" do
    ostruct = OpenStruct.new(args)
    ostruct.a
    ostruct.b
    ostruct.c
    ostruct.d
    ostruct.e
  end

  x.report "struct new" do
    struct.new(args)
  end

  x.report "struct acccess" do
    s = struct.new(args)
    s.a
    s.b
    s.c
    s.d
    s.e
  end

  x.compare!
end

Ruby 2.7.2

ruby 2.7.2p137 (2020-10-01 revision 5445e04352) [x86_64-linux]
Warming up --------------------------------------
      openstruct new   139.065k i/100ms
   openstruct access     7.325k i/100ms
          struct new   253.568k i/100ms
      struct acccess   204.764k i/100ms
Calculating -------------------------------------
      openstruct new      1.286M (± 4.1%) i/s -      6.536M in   5.090813s
   openstruct access     80.426k (± 2.7%) i/s -    402.875k in   5.013021s
          struct new      2.836M (± 2.9%) i/s -     14.200M in   5.011973s
      struct acccess      2.219M (± 4.2%) i/s -     11.262M in   5.085001s

Comparison:
          struct new:  2835598.8 i/s
      struct acccess:  2218940.2 i/s - 1.28x  (± 0.00) slower
      openstruct new:  1286202.5 i/s - 2.20x  (± 0.00) slower
   openstruct access:    80426.2 i/s - 35.26x  (± 0.00) slower

Ruby 3.0.0

ruby 3.0.0p0 (2020-12-25 revision 95aff21468) [x86_64-linux]
Warming up --------------------------------------
      openstruct new     9.826k i/100ms
   openstruct access     8.316k i/100ms
          struct new   320.240k i/100ms
      struct acccess   256.925k i/100ms
Calculating -------------------------------------
      openstruct new     98.372k (± 1.3%) i/s -    501.126k in   5.095116s
   openstruct access     83.194k (± 1.1%) i/s -    424.116k in   5.098618s
          struct new      3.219M (± 0.6%) i/s -     16.332M in   5.073638s
      struct acccess      2.544M (± 1.9%) i/s -     12.846M in   5.051045s

Comparison:
          struct new:  3219147.6 i/s
      struct acccess:  2544267.1 i/s - 1.27x  (± 0.00) slower
      openstruct new:    98372.2 i/s - 32.72x  (± 0.00) slower
   openstruct access:    83193.5 i/s - 38.69x  (± 0.00) slower

OpenStruct.new is much slower on Ruby 3.0.0 than Ruby 2.7.2

In Ruby 3.0.0 there's a new method cache mechanism so the statement "invalidates Ruby global method cache" in the cop description might not be true anymore.

Check-list

• Mention this proposal in the relevant Slack channels (e.g. #development, #backend, #frontend)
• [-] If there is a choice to make between two potential styles, set up an emoji vote in the MR:
  • CHOICE_A:
  • CHOICE_B:
  • Vote yourself for both choices so that people know these are the choices
• The MR doesn't have significant objections, and is getting a majority of vs (remember that we don't need to reach a consensus)
• [-] (If applicable) One style is getting a majority of vote (compared to the other choice)
• [-] (If applicable) Update the MR with the chosen style
• Follow the review process as usual

/cc @gitlab-org/maintainers/rails-backend