7 ms·

A potential lesson here (i.e. I am applying confirmation bias to retroactively view this article as justification for a strongly held opinion, lol): Unless you

A potential lesson here (i.e. I am applying confirmation bias to retroactively view this article as justification for a strongly held opinion, lol): Unless you are gonna benchmark something, for details like this you should pretty much always just trust the damn compiler and write the code in the most maintainable way. This comes up in code review a LOT at my work: - "you can write this simpler with XYZ" - "but that will be slower because it's a copy/a function call/an indirect branch/a channel send/a shared memory access/some other combination of assumptions about what the compiler will generate and what is slow on a CPU" I always ask them to either prove it or write the simple thing. If the code in question isn't hot enough to bother benchmarking it, the performance benefits probably aren't worth it _even if they exist_.

Dobbs 4y ago
edit: I misread the previous post. Ignore this. How are you using the word simpler? Because to me that implies a combination of more obvious and number of lines of code. Something that a benchmark shouldn't be involved in. For example asking someone to delete 10 lines of code and instead use go's ` net.SplitHostPort` would be an example of "simpler".
- 2OEH8eoCRo0 4y ago
  I've read that good generals worry about tactics and great generals worry about logistics. Good programmers play code golf, great programmers write readable and maintainable code. Your example seems reasonable but programmers also like to act like the smartest one in the room. I often come across tricky and borderline obfuscated code because somebody wanted to look clever. This is a logistical nightmare.
  - tested23 4y ago
    Ugh, you are right but then someone comes and uses this to rationalize not including things like map, filter and reduce in a language because they are supposedly too complicated and you can just do it with a for loop
    - baby 4y ago
      I work in a Rust codebase that uses a lot of functional functions, and I’ll say this: on average the imperative style takes less lines of code and less indentation. I also find it more readable personally, and idiomatic.
      - nicoburns 4y ago
        Functional iteration is good for the same reason we use for loops over while loops, and while loops over goto: they are more constrained, more clearly communicate intent, and are therefore easier to reason about.
        josephg 4y ago
        Sure but it’s easy to go overboard with this stuff. Reduce (fold) especially can be pretty hard to read in hairy situations. My general rule is that if you need fewer lines of code to implement your logic with a simple for loop, you probably should.
        nicoburns 4y ago
        Yeah, I agree with that. Especially reduce/fold, which I find is almost always better written as loop. Filter would be a good example of the opposite for me: almost always much clearer written functionally.
        jamincan 4y ago
        I find I frequently use a combination - use map/filter to setup an iterator, and then reduce it in a loop.
      - josephg 4y ago
        Just because we’re on the topic of performance: the rust optimizer can sometimes generate better code if you use map / filter / etc. The slice iterator in any context is a huge win over manual array iteration because it only needs to do bounds checking once. Javascript (v8, last I checked) is the opposite. Simple for loops almost always outperform anything else.
- karamanolev 4y ago
  That's what they're saying: if someone says anything more complicated is faster, they challenge them to benchmark it. Usually, it turns out whoever argues the "is faster" point doesn't bother to benchmark it and the simpler code-wise thing wins out. So yes - the benchmark goes to performance, simplicity is in lines-of-code, cyclomatic, "in the eye of the beholder" or whatever other metric you choose, but usually it's obvious.
Diggsey 4y ago
One neat thing here is that the compiler is aware of which types are `Copy` and not internally mutable (not contianing an `UnsafeCell`). For these types, passing `&T` and `T` are equivalent, so the compiler could just choose the faster option. Even if it's not smart enough to do that today, it could implement this optimization in the future. This could work even without inlining, since the Rust calling convention is unstable, and an optimization based on type size could be incorporated into it.
- zozbot234 4y ago
  It would be more advisable to add this as a clippy hint, because `&T` and `T` are not always equivalent wrt. FFI.
  - kibwen 4y ago
    Indeed, but the compiler is still capable of doing it on a case-by-case basis. Quite often the observed semantics are identical and it's easy for the backend to see that a pointer has been created only to be immediately dereferenced.
- cwzwarich 4y ago
  It would be nice if Rust could do this, but it breaks backwards compatibility. Some existing code depends on pointer values of &T being equal or not equal.
  - comex 4y ago
    As an addendum, LLVM can automatically perform the “&T-as-T” optimization (without inlining) in some cases where the callee function is in the same compilation unit and known to not care about the pointer value. However, these types of optimizations tend to be fragile, easily disturbed when things get slightly complex.
szundi 4y ago
My favorite way of thinking. Should be applied to question the need for the existence of the given feature or function too, and best to delete the whole stuff.
forrestthewoods 4y ago
Blog author here. I somewhat agree, somewhat disagree. This line makes me uneasy: > I always ask them to either prove it or write the simple thing. If the code in question isn't hot enough to bother benchmarking it, the performance benefits probably aren't worth it _even if they exist_. One of my philosophies is that death by a thousand cuts is fine, but death by ten thousand cuts isn’t. A team of 10 engineers can probably fix most of a thousand cuts in two or three months. But if you have ten thousand cuts you’re probably doomed. And those don’t show up cleanly in a flame graph. Now for some context my background is video games. Which means the team knows they need to hit an aggressive performance bar. This isn’t true for many projects. shared_ptr is a canonical example of death by ten thousand cuts. That said, I strongly agree with the principle of “just do the simple thing”. However I think it’s important to have “sane defaults”. A project can easily have a thousand or ten thousand papercuts that kill performance. But you can’t microbench every tiny decision. And microbenches are only a vague approximation of what actually matters. I’m also wary of “the compiler will make it fast”. Because that’s true… until it’s not! Although these days you don’t have any choice but to lean heavily on the compiler and “trust but verify”. No one wants a super complex solution if it’s not needed. However I am very amenable to “do a slightly more complex thing if you know it’s correct and we can never think about this ever again”. It’s much easier to do the fast thing upfront than for someone else to try and speed it up in two years when we’re doing a papercut pass.
- klyrs 4y ago
  > But if you have ten thousand cuts you’re probably doomed. And those don’t show up cleanly in a flame graph. I am reminded of the lovely nanosecond/microsecond talk by Grace Hopper. If your code does a little bit of setup and then spends all of its time in a single hotspot, fine. But if your code is full of microsecond-suboptimal speed bumps, you can probably hide your hotspot altogether. And a flat-ish flame graph looks fine: nothing stands out as a problem! It's valuable to do micro-benchmarks, not just to hone your optimization skills, but to learn optimal patterns in your language of choice. Then, when you're "in the zone" and laying down new code, you just do the optimal thing reflexively. Or, when you're reviewing or rewriting something, those micro-hotspots jump out and grab your attention. There's a reason that ancient software running on ancient hardware is way more responsive & snappy than what we have today. Laziness.
Patrol8394 4y ago
This x 10000 ! If I had a dime for every time I provided this exact feedback in code reviews…I find surprising that a lot of devs in tech industry are obsessed with pointless micro optimizations and they don’t care about writing maintainable, testable simple code. My final comment is always to not outsmart compilers/jvm because they tend to do a much better job that developers. Please, don’t optimize unless you have reasons to do so and numbers backing that up.
- dahfizz 4y ago
  It depends on your specialization, I guess. If you're making a website, a few microseconds here and there probably don't matter. But in my field (Fintech), performance really does matter. Doing the simple, slow thing is just lazy and won't make it through review.
  - imron 4y ago
    Great. Should be easy to prove then.
    - dahfizz 4y ago
      Yup, we have a good benchmarking suite to make sure changes don't cause regressions and that optimization changes actually work. That said, I think asking a developer to write everything they do twice so that they can A/B test is overboard. You can come back and really aggressively optimize later, but I think the "default" should be the fast thing, rather than the slow & easy thing.
      - bmacho 4y ago
        If "performance really does matter" (in Fintech) then a developer surely can write everything twice or more.
        dahfizz 4y ago
        We could, yeah. I'm sure you're capable of re-writing everything you do twice. It's just a huge waste of time. You get just as much benefit by assigning a performance refactor to an engineer when needed vs literally halving or worse the whole teams productivity.
throwaway894345 4y ago
I generally agree, but it’s also not obvious to me in Rust (or in Go) whether passing by reference or by copy is more maintainable or clear. I guess what I want is some guidance on what I should do by default, which you sort of give with “do what is more maintainable”, but I can’t tell what that means in practice (I’ve been told to default to pass-by-reference in the past because most traits take &self and not self).
- bjackman 4y ago
  Yeah totally agree it's not always/usually obvious. But there are cases where there's a clear readibility/assumed-performance tradeoff and in those cases I say always prefer readibility (unless you benchmark).
- saghm 4y ago
  I don't think one of them is more clear or maintainable universally, but in a lot of contexts, there might be an obvious choice. As a trivial example (that isn't quite fair given that the topic is about structs), it will almost never be more clear or maintainable to pass a shared reference to an integer (although there may be cases where a mutable reference might make sense). I don't think there's much need for one to have precedence over the other by default; if anything, I see the discussion about performance tradeoffs not being worth fretting about in the absence of actual measurement to be an argument _against_ one of them being inherently preferable.
- kibwen 4y ago
  > I’ve been told to default to pass-by-reference in the past because most traits take &self and not self This is only blanket advice for designing traits, because as the trait author you don't know what concrete type the downstream user is going to want to use, and taking `&self` in that circumstance is the choice that is friendliest to both Copy and non-Copy types. If you're just writing a non-generic function and you do know what concrete types you're using, the flowchart is pretty simple: 1. If the type is not Copy, then pass by-ref if you just need to read the value, pass by-mutable-ref if you just need to mutate the value, and pass by-value if you want to consume the value. 2. If the type is Copy, then pass by-value, but if your type is really big or if benchmarking has determined that this is a critical code path then pass by-ref.
jjice 4y ago
> but that will be slower because it's a copy/a function call/an indirect branch/a channel send/a shared memory access I really dislike these takes. I see engineers optimize these cases and then go ahead and make two separate SQL queries that could be one, ruining any false optimization gains they got by lord know how many times. Yeah, you can loop over that 100 element list twice doing basic computation if you want, it's not going to make a difference for many engineering workloads, but could make a big difference in readability.
nextaccountic 4y ago
> I always ask them to either prove it or write the simple thing. Even if they do, they also need to make a case that in this specific case, performance matters enough to pessimize code simplicity and maintainability. Also, if performance is that critical, it's imperative to benchmark again after each compiler release to guard against codegen regressions. And benchmark after changing this piece of code. Otherwise, we can say that performance doesn't really matter.
- eldenring 4y ago
  I think you're missing the point of the previous comment, they are saying a good proxy for it being worth it to optimize, is if you're willing to benchmark it.
heydenberk 4y ago
Even if you do benchmark something, maintainability can be more important than a marginal performance improvement. I've seen this happen a lot with JavaScript, particularly in the last 5-10 years as JS engines have developed increasingly sophisticated approaches to performance. Today's optimization can be tomorrow's de-optimization. Even given an unchanging landscape of compiler/interpreter, tightly-optimized code can become de-optimized when updated and extended, as compared to maintainable code that may not suffer much performance degradation upon extension.
saghm 4y ago
This seems to be an unpopular opinion, but I feel similarly about how sometimes people seem to toss out `inline` (and even more suspect, `inline(always)` annotations on Rust functions like candy on Halloween and there are almost never any sort of actual measurements of whether it actually helps in the cases it's used. It's not even that I think it really hurts that much in most of the stuff I've worked on (which tends to be more sensitive to concurrency design and network round trips), but I can't help but worry that people using stuff like this when they don't seem to fully grasp the implications is a recipe for trouble.
- bjackman 4y ago
  Yeah inline is an absolute classic for this. The number of uncommented __attribute__((always_inline))s I see in C code drives me crazy. There are absolutely legitimate reasons to use that attribute but there should ALWAYS be a comment about why, so that later readers know in what conditions they can safely remove it.
  - phkahler 4y ago
    In the old days I saw C code with massive overuse of the "register" keyword. Meaning this variable should be kept in a register. Someone had ove 12 of these in one function back when that was waaaay more variables then processors had registers. Good thing modern register allocation by compilers makes this irrelevant.
    - int_19h 4y ago
      As I recall, "register" was already a no-op in C compilers by the 90s or so (maybe except for embedded? I just don't know enough about that).
zaphirplane 4y ago
> I always ask them to either prove it or write the simple thing This is cumulative thou, 1 may not make much difference. 100,000 of those will. There is that story of chrome slows down that was does discovered to be in strings each one of which is too little to make a difference

Dobbs 4y ago

edit: I misread the previous post. Ignore this. How are you using the word simpler? Because to me that implies a combination of more obvious and number of lines of code. Something that a benchmark shouldn't be involved in. For example asking someone to delete 10 lines of code and instead use go's ` net.SplitHostPort` would be an example of "simpler".

2OEH8eoCRo0 4y ago
I've read that good generals worry about tactics and great generals worry about logistics. Good programmers play code golf, great programmers write readable and maintainable code. Your example seems reasonable but programmers also like to act like the smartest one in the room. I often come across tricky and borderline obfuscated code because somebody wanted to look clever. This is a logistical nightmare.
- tested23 4y ago
  Ugh, you are right but then someone comes and uses this to rationalize not including things like map, filter and reduce in a language because they are supposedly too complicated and you can just do it with a for loop
  - baby 4y ago
    I work in a Rust codebase that uses a lot of functional functions, and I’ll say this: on average the imperative style takes less lines of code and less indentation. I also find it more readable personally, and idiomatic.
    - nicoburns 4y ago
      Functional iteration is good for the same reason we use for loops over while loops, and while loops over goto: they are more constrained, more clearly communicate intent, and are therefore easier to reason about.
      - josephg 4y ago
        Sure but it’s easy to go overboard with this stuff. Reduce (fold) especially can be pretty hard to read in hairy situations. My general rule is that if you need fewer lines of code to implement your logic with a simple for loop, you probably should.
        nicoburns 4y ago
        Yeah, I agree with that. Especially reduce/fold, which I find is almost always better written as loop. Filter would be a good example of the opposite for me: almost always much clearer written functionally.
        jamincan 4y ago
        I find I frequently use a combination - use map/filter to setup an iterator, and then reduce it in a loop.
    - josephg 4y ago
      Just because we’re on the topic of performance: the rust optimizer can sometimes generate better code if you use map / filter / etc. The slice iterator in any context is a huge win over manual array iteration because it only needs to do bounds checking once. Javascript (v8, last I checked) is the opposite. Simple for loops almost always outperform anything else.
karamanolev 4y ago
That's what they're saying: if someone says anything more complicated is faster, they challenge them to benchmark it. Usually, it turns out whoever argues the "is faster" point doesn't bother to benchmark it and the simpler code-wise thing wins out. So yes - the benchmark goes to performance, simplicity is in lines-of-code, cyclomatic, "in the eye of the beholder" or whatever other metric you choose, but usually it's obvious.

Diggsey 4y ago

One neat thing here is that the compiler is aware of which types are `Copy` and not internally mutable (not contianing an `UnsafeCell`). For these types, passing `&T` and `T` are equivalent, so the compiler could just choose the faster option. Even if it's not smart enough to do that today, it could implement this optimization in the future. This could work even without inlining, since the Rust calling convention is unstable, and an optimization based on type size could be incorporated into it.

zozbot234 4y ago
It would be more advisable to add this as a clippy hint, because `&T` and `T` are not always equivalent wrt. FFI.
- kibwen 4y ago
  Indeed, but the compiler is still capable of doing it on a case-by-case basis. Quite often the observed semantics are identical and it's easy for the backend to see that a pointer has been created only to be immediately dereferenced.
cwzwarich 4y ago
It would be nice if Rust could do this, but it breaks backwards compatibility. Some existing code depends on pointer values of &T being equal or not equal.
- comex 4y ago
  As an addendum, LLVM can automatically perform the “&T-as-T” optimization (without inlining) in some cases where the callee function is in the same compilation unit and known to not care about the pointer value. However, these types of optimizations tend to be fragile, easily disturbed when things get slightly complex.

szundi 4y ago

My favorite way of thinking. Should be applied to question the need for the existence of the given feature or function too, and best to delete the whole stuff.

forrestthewoods 4y ago

Blog author here. I somewhat agree, somewhat disagree. This line makes me uneasy: > I always ask them to either prove it or write the simple thing. If the code in question isn't hot enough to bother benchmarking it, the performance benefits probably aren't worth it _even if they exist_. One of my philosophies is that death by a thousand cuts is fine, but death by ten thousand cuts isn’t. A team of 10 engineers can probably fix most of a thousand cuts in two or three months. But if you have ten thousand cuts you’re probably doomed. And those don’t show up cleanly in a flame graph. Now for some context my background is video games. Which means the team knows they need to hit an aggressive performance bar. This isn’t true for many projects. shared_ptr is a canonical example of death by ten thousand cuts. That said, I strongly agree with the principle of “just do the simple thing”. However I think it’s important to have “sane defaults”. A project can easily have a thousand or ten thousand papercuts that kill performance. But you can’t microbench every tiny decision. And microbenches are only a vague approximation of what actually matters. I’m also wary of “the compiler will make it fast”. Because that’s true… until it’s not! Although these days you don’t have any choice but to lean heavily on the compiler and “trust but verify”. No one wants a super complex solution if it’s not needed. However I am very amenable to “do a slightly more complex thing if you know it’s correct and we can never think about this ever again”. It’s much easier to do the fast thing upfront than for someone else to try and speed it up in two years when we’re doing a papercut pass.

klyrs 4y ago
> But if you have ten thousand cuts you’re probably doomed. And those don’t show up cleanly in a flame graph. I am reminded of the lovely nanosecond/microsecond talk by Grace Hopper. If your code does a little bit of setup and then spends all of its time in a single hotspot, fine. But if your code is full of microsecond-suboptimal speed bumps, you can probably hide your hotspot altogether. And a flat-ish flame graph looks fine: nothing stands out as a problem! It's valuable to do micro-benchmarks, not just to hone your optimization skills, but to learn optimal patterns in your language of choice. Then, when you're "in the zone" and laying down new code, you just do the optimal thing reflexively. Or, when you're reviewing or rewriting something, those micro-hotspots jump out and grab your attention. There's a reason that ancient software running on ancient hardware is way more responsive & snappy than what we have today. Laziness.

Patrol8394 4y ago

This x 10000 ! If I had a dime for every time I provided this exact feedback in code reviews…I find surprising that a lot of devs in tech industry are obsessed with pointless micro optimizations and they don’t care about writing maintainable, testable simple code. My final comment is always to not outsmart compilers/jvm because they tend to do a much better job that developers. Please, don’t optimize unless you have reasons to do so and numbers backing that up.

dahfizz 4y ago
It depends on your specialization, I guess. If you're making a website, a few microseconds here and there probably don't matter. But in my field (Fintech), performance really does matter. Doing the simple, slow thing is just lazy and won't make it through review.
- imron 4y ago
  Great. Should be easy to prove then.
  - dahfizz 4y ago
    Yup, we have a good benchmarking suite to make sure changes don't cause regressions and that optimization changes actually work. That said, I think asking a developer to write everything they do twice so that they can A/B test is overboard. You can come back and really aggressively optimize later, but I think the "default" should be the fast thing, rather than the slow & easy thing.
    - bmacho 4y ago
      If "performance really does matter" (in Fintech) then a developer surely can write everything twice or more.
      - dahfizz 4y ago
        We could, yeah. I'm sure you're capable of re-writing everything you do twice. It's just a huge waste of time. You get just as much benefit by assigning a performance refactor to an engineer when needed vs literally halving or worse the whole teams productivity.

throwaway894345 4y ago

I generally agree, but it’s also not obvious to me in Rust (or in Go) whether passing by reference or by copy is more maintainable or clear. I guess what I want is some guidance on what I should do by default, which you sort of give with “do what is more maintainable”, but I can’t tell what that means in practice (I’ve been told to default to pass-by-reference in the past because most traits take &self and not self).

bjackman 4y ago
Yeah totally agree it's not always/usually obvious. But there are cases where there's a clear readibility/assumed-performance tradeoff and in those cases I say always prefer readibility (unless you benchmark).
saghm 4y ago
I don't think one of them is more clear or maintainable universally, but in a lot of contexts, there might be an obvious choice. As a trivial example (that isn't quite fair given that the topic is about structs), it will almost never be more clear or maintainable to pass a shared reference to an integer (although there may be cases where a mutable reference might make sense). I don't think there's much need for one to have precedence over the other by default; if anything, I see the discussion about performance tradeoffs not being worth fretting about in the absence of actual measurement to be an argument _against_ one of them being inherently preferable.
kibwen 4y ago
> I’ve been told to default to pass-by-reference in the past because most traits take &self and not self This is only blanket advice for designing traits, because as the trait author you don't know what concrete type the downstream user is going to want to use, and taking `&self` in that circumstance is the choice that is friendliest to both Copy and non-Copy types. If you're just writing a non-generic function and you do know what concrete types you're using, the flowchart is pretty simple: 1. If the type is not Copy, then pass by-ref if you just need to read the value, pass by-mutable-ref if you just need to mutate the value, and pass by-value if you want to consume the value. 2. If the type is Copy, then pass by-value, but if your type is really big or if benchmarking has determined that this is a critical code path then pass by-ref.

jjice 4y ago

> but that will be slower because it's a copy/a function call/an indirect branch/a channel send/a shared memory access I really dislike these takes. I see engineers optimize these cases and then go ahead and make two separate SQL queries that could be one, ruining any false optimization gains they got by lord know how many times. Yeah, you can loop over that 100 element list twice doing basic computation if you want, it's not going to make a difference for many engineering workloads, but could make a big difference in readability.

nextaccountic 4y ago

> I always ask them to either prove it or write the simple thing. Even if they do, they also need to make a case that in this specific case, performance matters enough to pessimize code simplicity and maintainability. Also, if performance is that critical, it's imperative to benchmark again after each compiler release to guard against codegen regressions. And benchmark after changing this piece of code. Otherwise, we can say that performance doesn't really matter.

eldenring 4y ago
I think you're missing the point of the previous comment, they are saying a good proxy for it being worth it to optimize, is if you're willing to benchmark it.

heydenberk 4y ago

Even if you do benchmark something, maintainability can be more important than a marginal performance improvement. I've seen this happen a lot with JavaScript, particularly in the last 5-10 years as JS engines have developed increasingly sophisticated approaches to performance. Today's optimization can be tomorrow's de-optimization. Even given an unchanging landscape of compiler/interpreter, tightly-optimized code can become de-optimized when updated and extended, as compared to maintainable code that may not suffer much performance degradation upon extension.

saghm 4y ago

This seems to be an unpopular opinion, but I feel similarly about how sometimes people seem to toss out `inline` (and even more suspect, `inline(always)` annotations on Rust functions like candy on Halloween and there are almost never any sort of actual measurements of whether it actually helps in the cases it's used. It's not even that I think it really hurts that much in most of the stuff I've worked on (which tends to be more sensitive to concurrency design and network round trips), but I can't help but worry that people using stuff like this when they don't seem to fully grasp the implications is a recipe for trouble.

bjackman 4y ago
Yeah inline is an absolute classic for this. The number of uncommented __attribute__((always_inline))s I see in C code drives me crazy. There are absolutely legitimate reasons to use that attribute but there should ALWAYS be a comment about why, so that later readers know in what conditions they can safely remove it.
- phkahler 4y ago
  In the old days I saw C code with massive overuse of the "register" keyword. Meaning this variable should be kept in a register. Someone had ove 12 of these in one function back when that was waaaay more variables then processors had registers. Good thing modern register allocation by compilers makes this irrelevant.
  - int_19h 4y ago
    As I recall, "register" was already a no-op in C compilers by the 90s or so (maybe except for embedded? I just don't know enough about that).

zaphirplane 4y ago

> I always ask them to either prove it or write the simple thing This is cumulative thou, 1 may not make much difference. 100,000 of those will. There is that story of chrome slows down that was does discovered to be in strings each one of which is too little to make a difference