Is there a preferred method to search for entities? My tests show close results, sometimes favoring .grep sometimes favoring .is_a?
Does anyone have any more definitive results?
# version 1
Sketchup.active_model.entities.grep(Sketchup::Face).each { |entity| puts entity.entityID }
# version 2
Sketchup.active_model.entities.each { |entity| puts entity.entityID if entity.is_a? Sketchup::Face }try without the .each on grep as it is an enumerator itself…
i.e.
Sketchup.active_model.entities.grep(Sketchup::Face) { |entity|
puts entity.entityID
}
john
Thanks John, you’re completely right. I still get about a 1-2% difference between each method.
Is one way preferred by other developers?
I often use both in a single method…
model = Sketchup.active_model
defs = model.definitions
ents = model.active_entities
ents.each do |e|
if (e.is_a? Sketchup::Group) || (e.is_a? Sketchup::ComponentInstance)
# do something with e...
puts "#{e}"
dents = e.definition.entities
nested = dents.grep(Sketchup::Group).concat(dents.grep(Sketchup::ComponentInstance))
nested.each do |child|
# do something with each ent inside e...
puts " #{child}"
end
end
end
john
1-2% difference is usually not noticeable. I would choose whatever requires the least amount of code and thus is easiest/fastest to read. Code readability is very important when you go back later to add more features or to fix bugs.
Great, so whichever makes the code readable. Thanks @john_drivenupthewall and @eneroth3 for the advice.
Of the code examples you provided I’d say the first is easier to read because there is less text in total and the code is divided into smaller chunks. You can very easily see what entities are iterated before having to care what are later done to those entities. For this example the difference isn’t very big but it’s good to form a habit of writing as readable code as possible when making larger projects.
Btw, when using methods that atke arguments such as is_a? it is recommended to have parenthesis around the arguments. This makes it slightly faster and easier for the brain to parse the code. Again, it’s not a huge difference but when the codebase is getting bigger all these small things add up.
Good point, I made an error like that before with something like:
# bad code below:
if e.is_a? Sketchup::Group || e.is_a? Sketchup::ComponentInstance
most readable is to use words and new lines when available…
if entity.is_a?(Sketchup::Group) or # instead of || + new line allows comments
entity.is_a?(Sketchup::ComponentInstance)
# do something with entity...
john
the keywords or and and are not recommended in Ruby since their priority can be quite unexpected. Also I wouldn’t use multiple rows for a condition since you could then easily read part of the condition as a part of the code block. For this example I would extract the condition to a separate method.
if is_instance?(entity)
# Do stuff
end
def is_instance?(entity)
entity.is_a?(Sekcthup::Group) || entity.is_a?(Sketchup::ComponentInstance)
end
The only definitive answer is to profile your code in the context of what you are trying to do.
There is an old thread on SketchUcation on the topic of performance optimisations; Optimization Tips • sketchUcation • 1.
One of the finding there was that for in loops was faster than .each (in general). But that held true for Ruby 1.8. In Ruby 2.0 and onwards that changed.
And there are many other subtle behaviours that could affect how fast your loop will be. So; profile, profile, profile.
@tt_su Any recommended profiling tools Thom? From looking at the Sketchup Github there was an abandoned TestUp tool. Couldn’t find anything else.
I heartily share this sentiment, with one caveat: by “least amount of code” I would mean the simplest, cleanest expression vs multiple things packed onto each line. Packing onto a line has a tiny effect as the interpreter parses the code, but no effect at all on run time. But, at least in my experience, it makes it harder for a human to understand the logic. I always choose ease of reading over terseness, provided of course that the expressions are equivalent. Many logic errors result from trying to be too terse or clever!
True. There is a large corelation between short code and easy to read code but not in all cases.
You are getting into stylistic issues so we will have to agree to disagree. I do use parenthesis around if statements etc as I find them very much easier to speed read and maintain.
I certainly agree that adding parenthesis to all methods helps the brain to learn.
I use the c style && instead of and and I use || instead of or. It speeds readability and there is no confusion on precedence.
I use single quotes for string literals when possible. It is slightly faster than double quotes as the parser doesn’t have to take the time to look for escaped characters etc. Having said that I do make use of << for string concatenations and “#{}” instead of printf style.
I also break up long conditional statements on to multiple lines and I create references to objects.
ents = Sketchup.active_model.entities
ents.grep(Sketchup::Face) { |ent| puts ent.entityID }
Having said that Additionally when workable I like to have case statements on the same line where possible and I do make use of white space not tabs to line everything up. If you are sharing code spaces always line up - tabs only line up if the reader has their tab settings set the same as you.
case txt[0] when 'type'; cab.type = s.to_i when 'style'; cab.style = s.to_i when 'description'; cab.description = s when 'note'; cab.note = s when 'model'; cab.model = s…
TestUp in the Developer Tools repo is abandoned. TestUp2 is live and active. However, TestUp is for running test units - not for profiling.
It’s also worth having a look at what the Ruby community in general use (We don’t reinvent the wheel for SketchUp.)
In many cases when we have a small snippet we want to compare then Benchmark can be used:
For actual profiling, of digging into a call stack and see what is using the most time I’ve been experimenting with the RubyProf gem. But it’s been a pain to get working because it need compiling. Additionally it stopped working properly in recent SU versions so I had to hack it to get useful data. Been hoping to get time to package up this into a SU tool.
The most common reasons for slow extension performance that I observer are:
model.start_operation to true..typename instead of .is_a? (Huge impact! Strings are slow, in Ruby; Very slow!)entities.erase_entities(array_entities) is faster than entities.to_a.each { |e| e.erase! }. Similarly, array_entities.each { |e| selection.add(e) } is slower than selection.add(array_entities). Always operate in bulk if possible.That’s a good point. I was also using quick-and-dirty:
t1 = Time.now()
# do some action
t2 = Time.now()
p t2 - t1Yea - that’s what I do for very quick tests. I reach for Benchmark when I formalize my tests - setting up tests that I can run to monitor performance in my applications over time.
When trying to benchmark code, I would try to remove any IO code, as its duration/timing may be longer than the code under test. Obviously, IO to a hard drive will be slow, but puts is also IO. Said simply, having a puts in your blocks invalidates the test results.
I believe timing resolution varies by OS, and Windows may only be 1 frarme, or 1/60 second (ruby trunk may change this, as to backports to 2.4 or 2.3, unknown). Multiples of 0.0156 are the give-away…
Timings vary, which is the reason most tests using benchmark perform the tests many times.
Generally, most methods for selecting from collections will be pretty close as to time, and whatever other code you’re using (ie, code interacting with SU objects) will probably be where the optimizations can occur. Complex calculations may also require optimization.
Below is some code I used to check some selection methods (changed to compact it)…
# frozen_string_literal: true
SKETCHUP_CONSOLE.clear if defined? SKETCHUP_CONSOLE
ents = Sketchup.active_model.entities
ary_each = []
ary_compact = []
ary_grep = []
ary_select = []
t1 = Time.now ; ents.each { |e| ary_each << e.entityID if e.is_a?(Sketchup::Face) }
t2 = Time.now ; ary_compact = ents.map { |e| e.is_a?(Sketchup::Face) ? e.entityID : nil }.compact
t3 = Time.now ; ary_grep = ents.grep(Sketchup::Face) { |e| e.entityID }
t4 = Time.now ; ary_select = ents.select { |e| e.is_a?(Sketchup::Face) }.map { |e| e.entityID }
t5 = Time.now
puts "#{t2 - t1} ents.each { |e| ary_each << e.entityID if e.is_a?(Sketchup::Face) }"
puts "#{t3 - t2} ary_compact = ents.map { |e| e.is_a?(Sketchup::Face) ? e.entityID : nil }.compact"
puts "#{t4 - t3} ary_grep = ents.grep(Sketchup::Face) { |e| e.entityID }"
puts "#{t5 - t4} ary_select = ents.select { |e| e.is_a?(Sketchup::Face) }.map { |e| e.entityID }"
ary_each.sort! ; ary_compact.sort! ; ary_grep.sort! ; ary_select.sort!
# test to see if results match
puts "\nary_each == ary_compact is #{ary_each == ary_compact}"
puts "ary_grep == ary_compact is #{ary_grep == ary_compact}"
puts "ary_grep == ary_select is #{ary_grep == ary_select}"
puts "\nTotal faces #{ary_each.length}"
When repeating the test, it was a wash, all methods took about the same amount of time, which was less than 0.05 sec for a model with about 25k faces.
Greg