Slow Ruby Code Execution

Hi there!

I’m working on some automization in Sketchup and have met an unexpected problem when running my tested code in my extension.

When testing methods i like to run the code in Ruby Console+ to see the Sketchup methods through autocompletion. For optimization, i made a method for cleaning up a model by removing hidden elements, going from ~120000 elements to 45 in the scene.
This took a understandable 25s to complete, and all was swell.

Then i tried to run the same code from my extension, and it took 340s! Then i tried it form the ruby console in Sketchup, and it took about the same 340s to complete.

I know there’s probably something clever i can do to optimize the algorithm when cleaning, but is there some way to reach a similar speed to the ruby console+?

Thank you in advance for any help and shared wisdom on the subject. :slight_smile:

The method i’m now using is:

sams_extension_submenu.add_item("Cleanup model (should ideally take less than 60s)") do
  puts "Cleaning up model from #{Sketchup.active_model.entities.length} entities"
  time_start = Time.now
  i = 0   # Progress counter 
  Sketchup.active_model.entities.each{ |entity|
    if entity.hidden? == true
      entity.erase!
    end
    i += 1
    # Print progress every 1000 entities. Seems to have negligible impact on speed
    if i % 1000 == 0   
      puts "Sorted #{i} entities"
    end
  }
  time_end = Time.now
  puts "Took #{time_end - time_start}s to optimize"
end

(1) The number one rule to iterating (at least in Ruby) is to never modify (especially delete members of,) the collection as you are iterating it. This can cause the skipping of collection members.
Either make an array copy (#to_a) to iterate deleting the members from the original collection object,
Or collect all the references of the members to be deleted into an array and use the bulk Entities#erase_entities method.

(2) When you need an index with an iterator, use the #each_with_index method. This avoids having to write code statements to create an index variable and the increment statement.

(3) When changing the model entities, use an undo operation and switch off UI updates.


sams_extension_submenu.add_item(
  "Cleanup model (should ideally take less than 60s)"
) do
  model = Sketchup.active_model
  ents = model.entities
  doomed = ents.select(&:hidden?)
  unless doomed.empty?
    puts "Cleaning up model from #{ents.size} entities"
    puts "Erasing #{doomed.size} hidden entities."
    disable_ui = true
    time_start = Time.now
    model.start_operation("Erase Hidden Entities",disable_ui)
      ents.erase_entities(doomed)
    model.commit_operation
    time_end = Time.now
    puts "Took #{time_end - time_start}s to optimize"
  else
    puts "No hidden entities to erase at model's toplevel."
  end
end

EDIT: doomed = ents.select(&:hidden?) was erroneously doomed = ents.collect(&:hidden?)

1 Like

Wow, awesome! The .collect(&:hidden?) does not seem to be working as intended (i don’t really get it, could you point me to some documentation explaining the &:hidden? part?), but i replaced it with find_all() and now it works!
It takes 28.6s in ruby console+
28.999s in ruby console
and 35.6 from extension

Not quite sure why it takes longer from the extension, but other than me feeling slightly bad about the doomed entities, this is great and acceptable timewise :slight_smile:

Cheers!

Working method:

model = Sketchup.active_model
ents = model.entities
doomed = ents.find_all{ |ent| ent.hidden?}
unless doomed.empty?
  puts "Cleaning up model from #{ents.size} entities"
  puts "Erasing #{doomed.length} hidden entities."
  disable_ui = true
  time_start = Time.now
  model.start_operation("Erase Hidden Entities",disable_ui)
    ents.erase_entities(doomed)
  model.commit_operation
  time_end = Time.now
  puts "Took #{time_end - time_start}s to optimize"
else
  puts "No hidden entities to erase at model's toplevel."
end

I think Dan just made a little mistake here, I guess .select has been in his mind… :wink:
In your case the .select and - as you find out - .find_all doing the same thing here.

doomed = ents.find_all{ |ent| ent.hidden?}
# ... can be written as :
doomed = ents.find_all(&:hidden?)
# or
doomed = ents.select{ |ent| ent.hidden?}
doomed = ents.select(&:hidden?)

It is a relatively new shorthand syntax…

2 Likes

Yes true. Wrote the wrong method name.
collect and map are aliases.
select and find_all are aliases.

(And I fixed the code block above since it’s marked as a solution.)

1 Like

BTW. Just for completeness.
I read it recently and it is not clear yet in my incomplete programming knowledge, but there is something more to do with Enumerable#select / Array#select vs Hash#select:blush:

I think, in the case of Array and Hash their own methods override the one mixed in from Enumerable.

Certainly Hash needs to override #select, since the block receives two arguments (| key, value |) and returns a Hash, whereas Enumerable#select’s block receives only one argument and returns either an Array or if no block is given, an Enumerator. It’s not clear to me from the Ruby docs whether Array simply uses the version from Enumerable, but it seems likely.

If you look at the C source you’ll see they differ.