Comment by 20k
6 hours ago
Its a strong signal that developers make mistakes, code review has empirically been shown to be one of the best ways of uncovering defects. If you aren't finding bugs that people have written during code review (or you're not doing code review at all like the person I was replying to!), they're slipping rightwards
Bugs usually include business logic edge cases, and significant problems include someone realising while reading the PR that we can actually create a much better solution to the underlying problem. Ideally that realisation would happen prior to the PR being offered, but that's also not really how humans work
Example: During a PR review for a graphical feature for a game, someone reading it realises that we can actually have a significantly better solution to the underlying problem. You can't catch this in testing, because it doesn't even make sense conceptually to test it. It also sucks that it happened after someone put in a lot of work, but with graphics development you expect a lot of what you write to get canned and replaced with a better solution, because the technology evolves over time. The work is iterative towards the final goal anyway
Its also very common to miss subtle edge cases with graphics hardware, eg someone misunderstood the intricacies of GPU hardware, or a team member has relevant experience that someone else does not have. Or they missed a problematic memory access pattern on some hardware for example. Or something simple like they've technically forgotten a barrier, that the validation layer doesn't report for some reason
Eg: The function "tanh" is broken on some AMD GPU hardware, and should never be used under any circumstances. The actual GPU implementation of it is just screwed. Ideally everyone would know this, but its a very common function to crop up during specific graphics algorithms (as it smoothly remaps the range [-inf, +inf] -> [-1, 1]). So occasionally I've spotted that, and had to explain that we need to use an approximation instead, and then now everyone knows . It rarely gets caught during testing setups, because people don't know they need to include that hardware in their tests in the first place
No comments yet
Contribute on Hacker News ↗