I’m just curious about which is the most efficient way of doing this kind of node enumiration:
for i in something():
o=[var1,var2,var3,varN][i]
o.new()
o.do_something_based_on_number_of_loops()
add_child(o)
or
for i in something():
match i:
0:
o=var1
o.new()
o.do_something_based_on_number_of_loops()
add_child(o)
1:
o=var2
o.new()
o.do_something_based_on_number_of_loops()
add_child(o)
2:
o=var3
o.new()
o.do_something_based_on_number_of_loops()
add_child(o)
N-1:
o=varN
o.new()
o.do_something_based_on_number_of_loops()
add_child(o)
or
var items = [var1,var2,var3,varN]
for i in something():
o=items[i]
o.new()
o.do_something_based_on_number_of_loops()
add_child(o)
Or is there a more efficient way of doing it?
Edit: Sorry if that wasn’t clear. Is it better to constantly get something from an “unstored list”, store the list in a variable, or not use a list and use a match statement instead? Do they have any advantages/disadvantages that make them better in certain situations?
Rule #1 of programming: Write good code first, then measure performance.
Rule number 2: stop dismissing performance questions just because of something some guy said decades ago. Performance matters, learning about performance matters, and answers like yours dont help anyone.
Did they ask if they should optimize, or did they ask which one generates more performant assembly? Which one of those questions did you answer?
Maybe they already measured and already knows this is a bottleneck. Maybe they are curious if match statements are a slow abstraction (e.g. in python, it’s essentially a chain of if/else. In rust it’s often compiled to an indexable table). Maybe the given example code is only partially representative of the actual code this is being applied to.
It’s so irritating to look up performance-related questions when this answer is at the top (and middle, and bottom) of every thread. I swear half the reason every piece of modern software runs like shit is because nobody bothered to learn how to optimize and now everyone just parrots that phrase instead of saying “i dont know”.
There’s tons of little “premature” optimizations that you can do that arent evil. Choosing the right data structure (how random is the access? Are you using keys? Does it need to be sorted?). Estimating time complexity and load size (e.g. “i’m parsing [11 million | 2] files, i should probably [keep time complexity in mind | ignore time complexity completely]”). Structuring loops in a way that’s easy for compilers to auto-vectorize - usually it’s not any harder to read what the loop is doing, so why not do it right away?
Yes i’m bitter =(
Use:
items=[...] for o in items: ...
This is the most direct way of doing what you want. The first option might allocate a new array each iteration, which is unnecessary. The match statement is both a pain in the ass to write and less direct, which at best compiles to the same thing and at worst has you doing a bunch of totally unneeded comparisons.
If this ‘i’ variable you used isn’t just an incrementing counter, use the last option. If it is though, it’s an extra counter you don’t actually need.
The performance difference here would be so small I doubt you could even observe it. So, you really shouldn’t worry about this particular pattern. Compiler optimizations are more likely to trigger on simple, direct code, so writing it as directly as possible is probably the fastest option anyway.
What is the purpose of the loop?
Speaking generally, I’d prefer the first option as long as ‘i’ is actually an index or other valid key. I’m not sure what the overhead is in godot, but in general you should avoid conditional statements when you have a direct access method like a key or index.
I don’t think you need the for loop in the last one
It was a loop for all of them. Sorry, I thought that was implied. I edited the post to make it more clear.
- They don’t do the same thing so I’m assuming you’re trying to iterate over something like in n3
- n3 should theoretically be the fastest as there’s only one allocation and less branches
- Time it with
get_ticks_usec()
over a significant number of samples https://docs.godotengine.org/en/stable/classes/class_time.html#class-time-method-get-ticks-usec - None of this will matter because GDScript is interpreted and the execution times vary wildly based on how the moon is aligned
- Premature optimization, yada yada
For the first case, it’s probably better to use
Dictionary
instead of a match on an array, or matrix as you presented.var dictio = {"key":value, "key2":value2}
Unless you’re dealing with over 1k elements, any performance difference between Dict and Array might as well be a margin of error. https://docs.godotengine.org/en/stable/classes/class_dictionary.html
For that for you presented later, it should be
for i in items:
,i
then will point directly to the value in that position of the array.o = i
What you are doing, or attempting to, isn’t clear
At face value, none of those things are “apples to apples”, the first thing is a matrix or Vector2. The second is simply a match, which is “a series of if/elif”. Efficiency here comes mainly from the type of of the object being matched: bool is the fastest, int probably second, then float, then string. The third you’re just running through the array. All different things, each useful for a different purpose and none directly equivalent to another, thus it’s impossible to say which is the most efficient.
deleted by creator
I might be wrong, but the 2nd case looks like an anti pattern, the loop switch sequence .
The last case looks the most readable to me. Always start with that unless there’s a clear reason not to (eg inefficient multiple nested loops).
If speed is actually important then I would consider
writing it in another languagelooking for someone else’s implementation (personally). I try to focus on readability and doing the simplest implementation. I’ve been trying to use better function/variable names to explain what is happening at a glance instead of reading the whole thing. Have you considered using map for array iteration? I am hoping even if people don’t know how this works the intended result is even more readable than an equivalent for loop:func _show_only_first_layer_dots(): var set_dots_visibility = func(layer): layer.get_node("Dots").visible = (layer == $Layers.get_child(0)) $Layers.get_children().map(set_dots_visibility)
I find that much harder to read than a for loop. You are making a helper function to only use it once, which is kind of confusing when it is totally unnecessary. Also, distinguishing between two groups only inside the setter line is weird. Applying the modification to one group, then the other, is more obvious. Considering the alternative isn’t really longer, and only using basic loop syntax, I would just use the loop. If you really want to add the “set dots visibility” explanation into it, just use a comment, that’s what they’re for.
I literally just now misunderstood your code and had to change my comment to correct for it.
You looked at how it works and made comments about that (which I am thankful for and will enjoy replying to below). Can I first bring your attention to what is does instead of how it does it. Do you think that the names of the method function and helper function made it’s intended result clear? Could the use of map be so unexpected as to be a distraction?
I shouldn’t have included the node.visible = (expression) in the example, that was needlessly complex. It’s just what was in front of me (I do however find it appealing because it reduces the number of lines but that is another discussion).
While for loops are a popular method taught to every programmer early on then a for loop is easier to understand step by step. I do find occasions where a for loop just looks better but I keep finding that I better understand what my code is actually doing when I try to write a loop using functional programming methods.
func _show_only_first_layer_dots(): for c in $Layers.get_children(): c.get_node("Dots").visible = false $Layers.get_child(0).get_node("Dots").visible = true
Mines 10x more readable
and I saved a line of code.Simplicity is king.
If you’re working on the function then yes; everyone learns for loops fairly early on.
If you just need to know what it is intended to do then I would argue you didn’t need to read anymore than the function name. If you do look further then I’d argue just the name of the helper function was easier to read than the whole for loop.
It’s a poor name choice then, because it actually says less about what it’s doing than the main function does.
Besides, what is the point of “looking further” just to stop at another function name? Wouldn’t looking further imply the need to review the implementation?
Seeing another function divides the code into another subsection. In the example it’s the only one there but if more was added then you could choose where to focus your attention on the implementation.
In practice it turns out the method to make just the first element visible was redundant anyway. It would be made visible during the setup function that all elements call.