Rule #1 of programming: Write good code first, then measure performance.
this post was submitted on 30 Sep 2023
25 points (96.3% liked)
Godot
5909 readers
1 users here now
Welcome to the programming.dev Godot community!
This is a place where you can discuss about anything relating to the Godot game engine. Feel free to ask questions, post tutorials, show off your godot game, etc.
Make sure to follow the Godot CoC while chatting
We have a matrix room that can be used for chatting with other members of the community here
Links
Other Communities
- [email protected]
- [email protected]
- [email protected]
- [email protected]
- [email protected]
- [email protected]
- [email protected]
Rules
- Posts need to be in english
- Posts with explicit content must be tagged with nsfw
- We do not condone harassment inside the community as well as trolling or equivalent behaviour
- Do not post illegal materials or post things encouraging actions such as pirating games
We have a four strike system in this community where you get warned the first time you break a rule, then given a week ban, then given a year ban, then a permanent ban. Certain actions may bypass this and go straight to permanent ban if severe enough and done with malicious intent
Wormhole
Credits
- The icon is a modified version of the official godot engine logo (changing the colors to a gradient and black background)
- The banner is from Godot Design
founded 1 year ago
MODERATORS
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 =(
stop dismissing performance questions
I did not dismiss it, I said measure the performance yourself.
Performance matters, learning about performance matters
Which is why I said you should measure performance. It's no use waffling about unmeasurable performance gains.
Did they ask if they should optimize, or did they ask which one generates more performant assembly?
To be pedantic, GDScript is an interpreted language, and does not generate bytecode or assembly. This means that the code performance is highly dependent on runtime conditions, and needs to be measured in the place where it's used.
Maybe they already measured and already knows this is a bottleneck.
If they already measured, then they would know which one is faster, because they measured it.
I swear half the reason every piece of modern software runs like shit is because nobody bothered to learn how to optimize
This is unrelated to what I said, which is "you should measure your performance to see what you need to optimize".
There’s tons of little “premature” optimizations that you can do that aren't evil.
And all of these optimizations are just as effective after you measure them to see if they're needed, and they're no longer premature.
Estimating time complexity and load size
Accurately estimating the performance impact of a design choice means the optimization is no longer premature. The rule-of-thumb is about using optimizations without taking appropriate time to their overall performance benefit. The particular question asked by the OP is very very unlikely to have any significant performance impact at all, unless it's in an extremely hot loop running millions of times per frame, at which point you should measure it to see which one is faster in your use case.
I did not dismiss it, I said measure the performance yourself.
Then why does anyone ask anything? Just figure it out yourself. Oh you read a book or went to college? Why? Should have just reinvented computers yourself man. Taking advantage of collective knowledge is for suckers /s
This means that the code performance is highly dependent on runtime conditions, and needs to be measured in the place where it's used.
How is that helpful for OP? For example, if his question was about rust i'd say "options 1 or 2 should be identical for speed", but if it's python i'd say "match statements are just chained if/else chains, so a direct array index would be faster". For another example, in python attribute access is a function call. x.y in a loop is slower than assigning z = x.y outside of the loop and calling z in the loop.
You can absolutely generalize and have rules of thumb for performance.
If they already measured, then they would know which one is faster, because they measured it.
Measurements can have unintuitive results based on the dataset used (which, for benchmarks, usually end up being artificial datasets). OP's measurements may not have been consistent with their working understanding, thus they ask outside sources to confirm the truth. Idk why you cant just give them the benefit of the doubt and like... answer the question they actually asked? The explanations for "why" that accompany that answer can also be incredibly helpful.
And all of these optimizations are just as effective after you measure them to see if they're needed, and they're no longer premature.
That implies that these optimizations are harder than doing it the "mundane" way. They're not.
Here's a fun micro optimization for compiled languages: on modern CPUs
x = x * (arr[0] * arr[1]) in a loop has better performance characteristics than
x = (x * arr[0]) * arr[1]
even though they do the same thing (in short, it's because of the data dependencies for out-of-order execution - compilers wont make this optimization automatically for floats). How much harder is it to write the first one compared to the second one? How much harder to read is the first compared to the second?
So why would you not just make the first one your default? Now all your future uses of that pattern will perform better, for no extra effort except the amortized cognitive fee of changing your default option.
Look at OPs question. Could the answer fall under a rule of thumb that they can apply as their default option for a scenario? I'm pretty sure it can. So who cares about "premature" or not?
The particular question asked by the OP is very very unlikely to have any significant performance impact at all, unless it's in an extremely hot loop running millions of times per frame
So instead of answering their question, you assume it isnt impacting performance and they're just asking for no reason?
I literally had this exact question about python like 8 months ago. I had a file parser that needed to process different chunks based on a tag. Performance was critical, several thousand files, each ~3mb), the tag dispatch happened about 100,000-300,000 times per file. The original was implemented with if/else. I switched it to match because i thought it was faster, it wasnt. I looked at a lot of threads with answers like yours until stumbling upon dictionary dispatch (i.e. key = tag, value = first class function to call on the tag's data) and array dispatch.
That change alone was a 15% performance improvement.
You have no idea how their program works, what their hot loop is, if they're just asking out of curiosity, whatever. Just answer their fuckin question my dude. Platitudes are a waste of everyone's time.
I'm with you. "Measure it yourself" is a bit of a non-answer. Sure, being able to measure things yourself is also a useful skill, but asking questions and having people with experience answer them is largely the point of places like this.
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?
- 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
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.
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.
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 language~~ looking 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.