3 ms·

Don't get me wrong, I do use the profiler a ton, and it's the first thing i reach for when i start noticing issues, but having that little comment about why i d

Don't get me wrong, I do use the profiler a ton, and it's the first thing i reach for when i start noticing issues, but having that little comment about why i did it that way lets me relax a bit. I'd never mark it as a TODO or something that would clutter a tool up with crap, but having it there could help. A comment out of something I did recently where it's a loop that iterates over an array that's expected to be a few hundred long, and each iteration loops over the array again: // This is O(n^2) out of pure laziness. Might cause issues if the array gets really big. It's not hurting anyone, took like 10 seconds to write, and if it ever becomes a problem, the first thing i'll see is that in the function docblock which lets me (or someone else) know that there is no technical reason it needed to be O(n^2).

tlarkworthy 10y ago
Profilers help identify hot spots, but don't really explain the dynamic behaviour of why something is a hot spot. So comments saying that something is O(n^2) when it could be O(n) is very helpful when you see the inner function hogging 90% of resources. Profilers don't get complexity classes.
- mattmanser 10y ago
  Not really, comments like this are often wrong or misleading. After all, the programmer hasn't actually tested anything, they're just making assumptions. Otherwise they'd have actually fixed it. It's just pointless noise. A profiler will tell you that a particular function was run 10,000 times costing 0.5ms per run. You're not going to have a problem tracking down the problem, the comment won't help.
macspoofing 10y ago
>// This is O(n^2) out of pure laziness. Might cause issues if the array gets really big It just feels lazy. You're passing the buck to another dev who may be working on the code, weeks, months or years down the line. What are they supposed to do about it? Randomly rewrite working code and risk introducing a bug? If you think the input might reasonably scale to a size that will cause an issue then do some preemptive defensive coding and fix it. Otherwise don't. Stand by your decision. TODOs and FIXMEs tend to linger and get ignored. If I'm looking for performance problems, I'm not scanning for source code TODOs. I load a profiler and I'll see which methods take too much time.
- Klathmon 10y ago
  >It just feels lazy. You're passing the buck to another dev who may be working on the code, weeks, months or years down the line. What are they supposed to do about it? Randomly rewrite working code and risk introducing a bug? Yes. If it's a performance problem weeks, months, or years down the road, the developer then is supposed to rewrite it, possibly introducing bugs, but also solving the performance problems that manifested in the past weeks, months, or years. >If you think the input might reasonably scale to a size that will cause an issue then do some preemptive defensive coding and fix it. Otherwise don't. I don't know about you, but I'm not omniscient. I can make some pretty good guesses, but I can't predict the future. Optimizing the code now to be super efficient when I'm pretty sure it won't be necessary would be a massive waste of time. But on the off chance it is necessary, I'd like to give a hint to myself (or another dev) about why I did what I did, and give some assurances that it's a place that can be pretty easily optimized if needed. >If I'm looking for performance problems, I'm not scanning for source code TODOs. I load a profiler and I'll see which methods take too much time. Again, I'm not leaving TODOs I'm leaving comments explaining the code a little. But I completely agree with that whole statement, and it's exactly what I do as well. But when I get to a method that is taking too much time, I can now see a small comment saying it's running in O(n^2) and there is no technical reason why it needs to. Now I can spend less time checking if it needs to run that slowly, and can get right into fixing it. The handful of bytes taken up by that comment, and the few seconds it took me to write it isn't going to make anything worse for anyone, and it might make life easier for someone in the future, so i'll do it.
  - macspoofing 10y ago
    >I can now see a small comment saying it's running in O(n^2) and there is no technical reason why it needs to. What technical reason?!!? That was my point! In your comment there is an implicit assumption that an O(N^2) algorithm is a potential problem simply because it is an O(N^2) algorithm. That's just wrong. Would you put a comment like: "This is O(n). Might cause issues if the 'c' gets too big"? It's useless. __IF__ it is a potential problem, your laziness simply created a time-bomb that will blow up in somebody's face later. Your comment isn't helping anybody. For example, when your product is new, you may have 10 users, taking 10 courses in various permutations. You write your naive O(N^2) search and everything looks great, and you put your comment in. Everything is great for months, then you hit it big, and now you have 100 courses and 10,000 users and now you're dead. Thanks for your comment, it was really helpful! Devs tend to be allergic to O(N^2) because it is beaten into us in school and during interviews. On the one hand, that's good, it gets people thinking about runtime performance. On the other hand, if you just have a shallow view of it it's a crutch. Those same people may for example, wrap that algorithm in an a lambda that has an internal time-complexity of O(N^2) anyway (but at least you don't see the double for-loop) or use a chain of lambda calls, getting their O(N) but nevertheless losing performance because of constant costs around execution and allocation. >and it might make life easier for someone in the future, so i'll do it. That's where I disagree. It really doesn't.

tlarkworthy 10y ago

Profilers help identify hot spots, but don't really explain the dynamic behaviour of why something is a hot spot. So comments saying that something is O(n^2) when it could be O(n) is very helpful when you see the inner function hogging 90% of resources. Profilers don't get complexity classes.

mattmanser 10y ago
Not really, comments like this are often wrong or misleading. After all, the programmer hasn't actually tested anything, they're just making assumptions. Otherwise they'd have actually fixed it. It's just pointless noise. A profiler will tell you that a particular function was run 10,000 times costing 0.5ms per run. You're not going to have a problem tracking down the problem, the comment won't help.

macspoofing 10y ago

>// This is O(n^2) out of pure laziness. Might cause issues if the array gets really big It just feels lazy. You're passing the buck to another dev who may be working on the code, weeks, months or years down the line. What are they supposed to do about it? Randomly rewrite working code and risk introducing a bug? If you think the input might reasonably scale to a size that will cause an issue then do some preemptive defensive coding and fix it. Otherwise don't. Stand by your decision. TODOs and FIXMEs tend to linger and get ignored. If I'm looking for performance problems, I'm not scanning for source code TODOs. I load a profiler and I'll see which methods take too much time.

Klathmon 10y ago
>It just feels lazy. You're passing the buck to another dev who may be working on the code, weeks, months or years down the line. What are they supposed to do about it? Randomly rewrite working code and risk introducing a bug? Yes. If it's a performance problem weeks, months, or years down the road, the developer then is supposed to rewrite it, possibly introducing bugs, but also solving the performance problems that manifested in the past weeks, months, or years. >If you think the input might reasonably scale to a size that will cause an issue then do some preemptive defensive coding and fix it. Otherwise don't. I don't know about you, but I'm not omniscient. I can make some pretty good guesses, but I can't predict the future. Optimizing the code now to be super efficient when I'm pretty sure it won't be necessary would be a massive waste of time. But on the off chance it is necessary, I'd like to give a hint to myself (or another dev) about why I did what I did, and give some assurances that it's a place that can be pretty easily optimized if needed. >If I'm looking for performance problems, I'm not scanning for source code TODOs. I load a profiler and I'll see which methods take too much time. Again, I'm not leaving TODOs I'm leaving comments explaining the code a little. But I completely agree with that whole statement, and it's exactly what I do as well. But when I get to a method that is taking too much time, I can now see a small comment saying it's running in O(n^2) and there is no technical reason why it needs to. Now I can spend less time checking if it needs to run that slowly, and can get right into fixing it. The handful of bytes taken up by that comment, and the few seconds it took me to write it isn't going to make anything worse for anyone, and it might make life easier for someone in the future, so i'll do it.
- macspoofing 10y ago
  >I can now see a small comment saying it's running in O(n^2) and there is no technical reason why it needs to. What technical reason?!!? That was my point! In your comment there is an implicit assumption that an O(N^2) algorithm is a potential problem simply because it is an O(N^2) algorithm. That's just wrong. Would you put a comment like: "This is O(n). Might cause issues if the 'c' gets too big"? It's useless. __IF__ it is a potential problem, your laziness simply created a time-bomb that will blow up in somebody's face later. Your comment isn't helping anybody. For example, when your product is new, you may have 10 users, taking 10 courses in various permutations. You write your naive O(N^2) search and everything looks great, and you put your comment in. Everything is great for months, then you hit it big, and now you have 100 courses and 10,000 users and now you're dead. Thanks for your comment, it was really helpful! Devs tend to be allergic to O(N^2) because it is beaten into us in school and during interviews. On the one hand, that's good, it gets people thinking about runtime performance. On the other hand, if you just have a shallow view of it it's a crutch. Those same people may for example, wrap that algorithm in an a lambda that has an internal time-complexity of O(N^2) anyway (but at least you don't see the double for-loop) or use a chain of lambda calls, getting their O(N) but nevertheless losing performance because of constant costs around execution and allocation. >and it might make life easier for someone in the future, so i'll do it. That's where I disagree. It really doesn't.