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

Use array instead of linked list for rbs location's child #1786

Conversation

pocke
Copy link
Member

@pocke pocke commented Apr 4, 2024

This PR reduces memory usage by replacing the linked list with an array for RBS::Location.

RBS::Location has child locations. They are implemented as two linked lists, but this is not memory efficient.
Therefore this PR replaces the linked lists with an array.

This PR is the first part of memory usage improvement for RBS::Location. I'll try reducing memory with another approach. See this document for more information. https://hackmd.io/QNkMU6roTqSn_iXb2ENhsA

Data structure

This is the previous data structure.
Comments are byte sizes (on my 64bits local machine).

typedef struct rbs_loc_list {
  ID name; // 8
  range rg; // 32
  struct rbs_loc_list *next; // 8
} rbs_loc_list; // 48

typedef struct {
  VALUE buffer; // 8
  range rg; // 32
  rbs_loc_list *requireds; // 8
  rbs_loc_list *optionals; // 8
} rbs_loc; // 56

By this change, these structs are changed to the following:

typedef struct {
  ID name; // 8
  range rg; // 32
} rbs_loc_entry; // 40

typedef unsigned int rbs_loc_entry_bitmap;

typedef struct {
  unsigned short len;  // 2
  unsigned short cap;  // 2
  rbs_loc_entry_bitmap required_p; // 4
  rbs_loc_entry entries[0]; // 0
} rbs_loc_children; // 8

typedef struct {
  VALUE buffer; // 8
  range rg; // 32
  rbs_loc_children *children; // 8
} rbs_loc; // 48

The byte size is changed like following:

  • If rbs_loc does not have children:
    • before: 56 bytes
    • after: 48 bytes
  • If rbs_loc has children:
    • before: 56 + (48 * size) bytes
    • after: 48 + 8 + (40 * size) bytes

Benchmarking

Memory Usage

With bundle exec ruby benchmark/memory_new_env.rb and benchmark/memory_new_rails_env.rb

before

$ bundle exec ruby benchmark/memory_new_env.rb
Total allocated: 43027240 bytes (363668 objects)
Total retained:  12315720 bytes (98104 objects)

retained memory by class
-----------------------------------
   4261392  String
   3899952  RBS::Location
   1262864  Array
   1086272  Hash
    257840  RBS::Types::Function

$ bundle exec ruby benchmark/memory_new_rails_env.rb
Total allocated: 224460224 bytes (1994661 objects)
Total retained:  57453656 bytes (553231 objects)

retained memory by class
-----------------------------------
  21650208  RBS::Location
  11885144  String
   7415928  Array
   6143008  Hash
   1481920  RBS::Types::Function

after

$ bundle exec ruby benchmark/memory_new_env.rb
Total allocated: 42677872 bytes (363668 objects)
Total retained:  11966352 bytes (98104 objects)

retained memory by class
-----------------------------------
   4261392  String
   3550584  RBS::Location
   1262864  Array
   1086272  Hash
    257840  RBS::Types::Function

$ bundle exec ruby benchmark/memory_new_rails_env.rb
Total allocated: 222420976 bytes (1994661 objects)
Total retained:  55414424 bytes (553231 objects)

retained memory by class
-----------------------------------
  19610976  RBS::Location
  11885144  String
   7415928  Array
   6143008  Hash
   1481920  RBS::Types::Function

The retained memory is reduced 3% (349kb) on the core env and 4% (2MB) on the Rails env.

Execution time

With bundle exec ruby benchmark/benchmark_new_env.rb.

before

Warming up --------------------------------------
             new_env     1.000 i/100ms
       new_rails_env     1.000 i/100ms
Calculating -------------------------------------
             new_env      6.146 (± 0.0%) i/s -     62.000 in  10.136343s
       new_rails_env      0.966 (± 0.0%) i/s -     10.000 in  10.377986s

after

Warming up --------------------------------------
             new_env     1.000 i/100ms
       new_rails_env     1.000 i/100ms
Calculating -------------------------------------
             new_env      6.399 (±15.6%) i/s -     64.000 in  10.070012s
       new_rails_env      1.013 (± 0.0%) i/s -     11.000 in  10.910335s

It is 1.04x faster on the core env and 1.05x faster on the Rails env.

@pocke pocke force-pushed the Use_array_instead_of_linked_list_for_RBS__Location_s_child branch from f00b016 to 18602a7 Compare April 5, 2024 02:18
@pocke pocke marked this pull request as ready for review April 8, 2024 07:22
@pocke pocke requested a review from soutaro April 8, 2024 08:53
@pocke pocke changed the title Use array instead of linked list for rbs location s child Use array instead of linked list for rbs location's child Apr 8, 2024
@soutaro soutaro added this to the RBS 3.5 milestone Apr 15, 2024
Copy link
Member

@soutaro soutaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@soutaro soutaro added this pull request to the merge queue Apr 15, 2024
Merged via the queue into ruby:master with commit 858fc21 Apr 15, 2024
17 checks passed
@pocke pocke deleted the Use_array_instead_of_linked_list_for_RBS__Location_s_child branch April 15, 2024 04:50
@soutaro soutaro added the Released PRs already included in the released version label Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Released PRs already included in the released version
Development

Successfully merging this pull request may close these issues.

2 participants