3 ms·

This week I had to deal with the exact thing you mention. A pull request for "performance optimization", basically a proposal to unwrap a higher abstraction to

This week I had to deal with the exact thing you mention. A pull request for "performance optimization", basically a proposal to unwrap a higher abstraction to do it at a lower level, manually and directly. Yes, it does save a few function calls and instantiating a class - but that's not where the real bottleneck is. The proposed changes would require rewriting large amounts of code elsewhere, making the logic more verbose and complex. It took some effort to convince the person that the trade-off isn't worth it. I tried to be diplomatic, but I'm afraid they took it personally that the pull request was declined. I wish I could have proved my objection with some metrics - how many milliseconds were shaved off by the proposal, versus the increased "cyclomatic complexity", as a measurement of its detrimental effect on the code base.

_pastel 6y ago
That burden of proof sounds backward; the person making the optimization should be benchmarking. Rather than rejecting outright, did you consider blocking the pull request and asking for benchmarks demonstrating that the optimization was worth the complexity increase? Maybe they would learn more from that process than from an explanation of why they are misguided.
- lioeters 6y ago
  Good point - I could have asked for a benchmark to prove that the optimizations are worth it. The person put effort into the code, and I didn't feel that they were convinced of my argument against it. Their changes were more efficient logically, and I'm sure it would have saved a few milliseconds per call - whereas my claim of increased complexity was a more subjective judgement. In the end, it came down to trust/authority - but they might have been more convinced, or felt better about the decision, if I could produce some numbers from my side, like a measurement of cyclomatic complexity based on static code analaysis. Perhaps that could have justified the existing higher abstractions, to weigh their (almost negligible) performance cost against the value of reduced complexity - keeping the code base simpler to understand.

_pastel 6y ago

That burden of proof sounds backward; the person making the optimization should be benchmarking. Rather than rejecting outright, did you consider blocking the pull request and asking for benchmarks demonstrating that the optimization was worth the complexity increase? Maybe they would learn more from that process than from an explanation of why they are misguided.

lioeters 6y ago
Good point - I could have asked for a benchmark to prove that the optimizations are worth it. The person put effort into the code, and I didn't feel that they were convinced of my argument against it. Their changes were more efficient logically, and I'm sure it would have saved a few milliseconds per call - whereas my claim of increased complexity was a more subjective judgement. In the end, it came down to trust/authority - but they might have been more convinced, or felt better about the decision, if I could produce some numbers from my side, like a measurement of cyclomatic complexity based on static code analaysis. Perhaps that could have justified the existing higher abstractions, to weigh their (almost negligible) performance cost against the value of reduced complexity - keeping the code base simpler to understand.