dendibakh / perf-ninja

This is an online course where you can learn and master the skill of low-level performance analysis and tuning.
2.44k stars 206 forks source link

Improve validation in loop_interchange_1 #7

Open dendibakh opened 3 years ago

dendibakh commented 3 years ago

Comment by @OleksandrKvl :

Current validation is strange in a way that you're using other functions from solution.cpp, not just the main power(). Ideally, you should have separate implementation inside validate.cpp, multiple some matrices with functions from validate.cpp and solution.cpp and just compare their results.

dendibakh commented 3 years ago

cc @andrewevstyukhin

andrewevstyukhin commented 3 years ago

Often in real cases not all code paths are covered. Any optimization can easily break something. It's a good honeypot in a too simple task to show reality. Anyway, commit 285f007e4f78bcc95bcdf97b25a7c631868ef66b prevents trival broken solution {}.

dendibakh commented 3 years ago

Often in real cases not all code paths are covered. Any optimization can easily break something. It's a good honeypot in a too simple task to show reality. Anyway, commit 285f007 prevents trival broken solution {}.

So Andrey, do you suggest that we don't duplicate code from solution.cpp into validate.cpp ?

andrewevstyukhin commented 3 years ago

I think we can use originalMultiply in validation.cpp to be more precise if current situation is so dramatic. Please try 285f007 first (just merge main into private/*)

OleksandrKvl commented 3 years ago

The problem is not just a validation, it's also about clear separation between client's and core code. Originally, I edited multiply() and identity() functions in my solution and was very surprised to find that they are used by someone else. I think that it should be clear what people can and cannot do. The simplest approach is to allow them to change only solution.h/cpp. For example existing solution.h:

// Assume this constant never changes
constexpr int N = 400;

// Square matrix 400 x 400
using Matrix = std::array<std::array<float, N>, N>;

void zero(Matrix &result);
void identity(Matrix &result);
void multiply(Matrix &result, const Matrix &a, const Matrix &b);
Matrix power(const Matrix &input, const uint32_t k);

void init(Matrix &matrix);

Should be just:

#include "problem.h" // Matrix and N are here now

Matrix power(const Matrix &input, const uint32_t k);
andrewevstyukhin commented 3 years ago

Performance problems lay in solution.h and solution.cpp. Microoptimizations don't mean to use other algorithm or rewrite source code. Location of the performance issue is also unknown. Good job is to use information from the topic and use a profiler. All issues are topmost, not second order.