производительность — я не понимаю, почему мой код на С ++ работает так медленно
Я работаю над маскировкой Собеля для обнаружения краев без использования какой-либо специальной библиотеки. Выходные данные, которые я хочу получить, представляют собой текстовые файлы с матрицей 512×512 со значениями от 0 до 1.
Я проверил, что код работает, поместив меньшие значения как 50 вместо ‘ROW-2’ и ‘COL-2’.
Однако, если я верну их обратно, выполнение кода будет длиться вечно.
Постоянные значения:
const int ROW = 512;
const int COL = 512;
const double Gx [3][3] = { {-1.0,0.0,1.0},{-2.0,0.0,2.0},{-1.0,0.0,1.0}};
const double Gy [3][3] = { {1.0,2.0,1.0},{0.0,0.0,0.0},{-1.0,-2.0,-1.0}};
Это основная функция:
int main()
{
double NewImage[ROW][COL] = {0};
for (int i = 0; i < ROW; i++)
{
for (int j = 0; j < COL; j++)
{
NewImage[i][j] = 0;
}
}
for (int i = 0; i < ROW-2; i++)
{
for (int j = 0; j < COL-2; j++)
{
NewImage[i+1][j+1] = SobelConvolution(i,j);
}
}
ofstream newImage;
string filename;
filename = "output image.txt";
newImage.open (filename.c_str());
for(int rows = 0; rows < ROW; rows++)
{
for(int cols = 0; cols < COL; cols++)
{
newImage << NewImage[ROW][COL] <<" ";
}
newImage << endl;
}
newImage.close();
return 0;
}
Это функция SobelConvolution:
double SobelConvolution(int row, int col)
{
double convX;
double convY;
double conv;
convX = ImageReader(row,col)*Gx[2][2]
+ ImageReader(row,col+1)*Gx[2][1]
+ ImageReader(row,col+2)*Gx[2][0]
+ ImageReader(row+1,col)*Gx[1][2]
+ ImageReader(row+1,col+1)*Gx[1][1]
+ ImageReader(row+1,col+2)*Gx[1][0]
+ ImageReader(row+2,col)*Gx[0][2]
+ ImageReader(row+2,col+1)*Gx[0][1]
+ ImageReader(row+2,col+2)*Gx[0][0];
convY = ImageReader(row,col)*Gy[2][2]
+ ImageReader(row,col+1)*Gy[2][1]
+ ImageReader(row,col+2)*Gy[2][0]
+ ImageReader(row+1,col)*Gy[1][2]
+ ImageReader(row+1,col+1)*Gy[1][1]
+ ImageReader(row+1,col+2)*Gy[1][0]
+ ImageReader(row+2,col)*Gy[0][2]
+ ImageReader(row+2,col+1)*Gy[0][1]
+ ImageReader(row+2,col+2)*Gy[0][0];
conv = sqrt((convX*convX) + (convY*convY));return conv;
}
Это функция ImageReader:
double ImageReader(int r, int c)
{
double OrigImage[ROW][COL];
ifstream defaultImage ("image.txt");
if (defaultImage.good())
{
for (int i = 0; i < ROW; i++)
{
for (int j = 0; j < COL; j++)
{
defaultImage >> OrigImage[i][j];
}
}
}
return OrigImage [r][c];
}
Любой намек или совет? Заранее спасибо!
Решение
Вот некоторые заметки:
-
ImageReaderВозвращает только одно значение массива, не нужно читать через все массив каждый раз вам просто нужно одно значение. На мой взгляд, эта функция избыточна.
-
SobelConvolutionЭта функция хороша, но есть ненужная переменная —
conv, -
mainЯ понятия не имею, почему вы инициализируете каждое значение
NewImageв0, когда они уже0! Вы также на самом деле не нуждаетесьNewImage
Вот что я бы написал (с подробными комментариями):
double SobelConvolution(int row, int col)
{
//ImageReader has been removed, it was unnecessary. The code has been moved here
double oldImage[ROW][COL];
std::ifstream defaultImage{ "image.txt" };
//Error handling if file doesn't exist - consider doing something else :)
if (!defaultImage.is_open())
return 0;
//Initialize array
for (int i = 0; i < ROW; ++i)
for (int j = 0; j < COL; ++j)
defaultImage >> oldImage[i][j];
//You should always declare variables where they are first used, this
//reduces the possibility of errors
//We can just access the array directly
double convX = oldImage[row][col] * Gx[2][2]
+ oldImage[row][col + 1] * Gx[2][1]
+ oldImage[row][col + 2] * Gx[2][0]
+ oldImage[row + 1][col] * Gx[1][2]
+ oldImage[row + 1][col + 1] * Gx[1][1]
+ oldImage[row + 1][col + 2] * Gx[1][0]
+ oldImage[row + 2][col] * Gx[0][2]
+ oldImage[row + 2][col + 1] * Gx[0][1]
+ oldImage[row + 2][col + 2] * Gx[0][0];
double convY = oldImage[row][col] * Gy[2][2]
+ oldImage[row][col + 1] * Gy[2][1]
+ oldImage[row][col + 2] * Gy[2][0]
+ oldImage[row + 1][col] * Gy[1][2]
+ oldImage[row + 1][col + 1] * Gy[1][1]
+ oldImage[row + 1][col + 2] * Gy[1][0]
+ oldImage[row + 2][col] * Gy[0][2]
+ oldImage[row + 2][col + 1] *Gy[0][1]
+ oldImage[row + 2][col + 2]*Gy[0][0];
//No need to create a separate variable just to return it
return sqrt((convX*convX) + (convY*convY));
}int main()
{
//= {} Initializes every element to 0, you don't need to do it :) Just so you know :)
//Note that it crashes here, because my stack size was too small,
//maybe consider using a dynamic array (512 * 512 is pretty big) :)
//double NewImage[ROW][COL] = {};
//The array is not really needed, see below
std::string filename = "oimage.txt";
std::ofstream newImage{ filename };
//No need to create another array just to output it again,
//Just output the calculated values - this doesn't ignore the first/last values
for (int rows = 0; rows < ROW; rows++)
{
for (int cols = 0; cols < COL; cols++)
newImage << SobelConvolution(rows, cols) << " ";
newImage << '\n'; //std::endl flushes the stream, while \n does not - it is faster :)
}
newImage.close();
return 0;
}
Другие решения
Вы действительно хотите открыть один файл изображения 18 раз и прочитать все данные для каждой строки и столбца, просто чтобы вернуть одну строку и столбец 18 раз? Почему бы не прочитать файл изображения один раз и передать массив данных изображения в функцию?
То, что вы делаете, не просто немного неэффективно, но, к сожалению, совершенно безумно.
Для каждого пикселя изображения вы вызываете SobelConvolution, который, в свою очередь, вызывает ImageReader 18 раз (из которых 6 бесполезны, так как соответствующий коэффициент равен нулю). Но ужасная вещь заключается в том, что ImageReader каждый раз выполняет полное чтение изображения из текстового файла, где достаточно простого поиска в массиве.
Таким образом, в общей сложности вы выполняете 4718592 открытие / закрытие файлового потока и считывание значения 1236950581248 из файла, где достаточно только 1 открытия / закрытия и 262144 считываний. (Не считая того, что одно чтение намного дороже, чем прямой доступ к массиву.) Полный цикл может длиться два часа или более.