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

polish finish_in_order #340

Merged
merged 1 commit into from
Dec 16, 2023
Merged

polish finish_in_order #340

merged 1 commit into from
Dec 16, 2023

Conversation

grosser
Copy link
Owner

@grosser grosser commented Dec 14, 2023

@shaicoleman
Copy link
Contributor

shaicoleman commented Dec 14, 2023

A nested array is inefficient, and requires significantly more memory allocations.
The rest of the changes look good to me

# gem install benchmark-memory
require 'benchmark/memory'

Benchmark.memory do |x|
  Pair = Struct.new(:a, :b)

  x.report('two arrays') do
   array_1 = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
   array_2 = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
 end
  x.report('hash') do
    hash = { 1 => 1, 2 => 2, 3 => 3, 4 => 4, 5 => 5, 6 => 6, 7 => 7, 8 => 8, 9 => 9, 10 => 10 }
  end
  x.report('nested array') do
    array = [[1, 1], [2, 2], [3, 3], [4, 4], [5, 5], [6, 6], [7, 7], [8, 8], [9, 9], [10, 10]]
  end
  x.report('struct') do
    array = [Pair.new(1, 1), Pair.new(2, 2), Pair.new(3, 3), Pair.new(4, 4), Pair.new(5, 5), 
             Pair.new(6, 6), Pair.new(7, 7), Pair.new(8, 8), Pair.new(9, 9), Pair.new(10, 10)]
  end
  x.compare!
end
Comparison:
          two arrays:         80 allocated
                hash:        512 allocated - 6.40x more
        nested array:        560 allocated - 7.00x more
              struct:        560 allocated - 7.00x more

@grosser
Copy link
Owner Author

grosser commented Dec 15, 2023

hehe nice! :D

I think it's not as easy to benchmark since we are doing lots of array expansions too,
but agree that most likely 2 arrays are the most efficient way ... just a bit ugly, I'll take another look

@grosser
Copy link
Owner Author

grosser commented Dec 16, 2023

I did not find a clean way of using less arrays.
being able to do break unless (done = options[:finish_done][i]) is kinda neat
otherwise I have to have 3 arrays, 1 for result, 1 for item, 1 for "done" which kinda sucks
and if I use a hash and check for key? then that's also more computation

... keeping it clean/simple until someone else has a nice idea or need for this 🤷

@grosser grosser merged commit 792a1fe into master Dec 16, 2023
7 checks passed
@grosser grosser deleted the grosser/order branch December 16, 2023 00:51
@grosser
Copy link
Owner Author

grosser commented Dec 16, 2023

1.24.0

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

Successfully merging this pull request may close these issues.

2 participants