Comments 27
Простите, не удержался.
8 сек. это слишком много. Мне кажется, у вас потоки борятся за ресурсы. Во время работы алгоритма, какая нагрузка на процессоры, случайно не 100%?
Эта статья — отличный пример неправильного использования многопоточности.
Вычислений почти нет, в основном работа с памтятью. Вот код, который на на 15% быстрее и выполняется в одном потоке:
Вычислений почти нет, в основном работа с памтятью. Вот код, который на на 15% быстрее и выполняется в одном потоке:
void contrastFilter(unsigned char * imageData, size_t dataSize, int contrast) // contrast (256 - normal)
{
unsigned char buf[256];
unsigned int midBright = 0;
for (size_t i = 0; i < dataSize; i += 3)
midBright += imageData[i] * 77 + imageData[i + 1] * 150 + imageData[i + 2] * 29;
midBright /= (256 * dataSize / 3);
for (size_t i = 0; i < 256; i++)
{
int a = (((i - midBright) * contrast) >> 8) + midBright;
if (a < 0) buf[i] = 0;
else if (a > 255) buf[i] = 255;
else buf[i] = a;
}
for (size_t i = 0; i < dataSize; i++)
imageData[i] = buf[imageData[i]];
}
Ничего себе вы уместили. А можете показать ошибки автора? У меня недостаточно опыта, чтобы самому их найти, но очень интересно. И ещё вопрос — почему и у вас и у автора коэффициенты
midBright += imageData[i] * 77 + imageData[i + 1] * 150 + imageData[i + 2] * 29;256 * dataSize>> 8Основная ошибка автора — использование распараллеливания там, где это не нужно. Вычислений почти нет, обращений к памяти много. Основное время процессор будет ждать передачи дынных, на сколько потоков это не раскидывай — лучше не будет.
Вместо 1.0 максимальная яркость принята за 256, поэтому константы почти такие же как у автора.
77 / 256 ~= 0.3
150 / 256 ~= 0.586
29 / 256 ~= 0.113
По поводу умножения и сдвига на 8 (деления на 256): да, константы для операций с фиксированной точкой могут показаться странными.
Вместо 1.0 максимальная яркость принята за 256, поэтому константы почти такие же как у автора.
77 / 256 ~= 0.3
150 / 256 ~= 0.586
29 / 256 ~= 0.113
По поводу умножения и сдвига на 8 (деления на 256): да, константы для операций с фиксированной точкой могут показаться странными.
UFO just landed and posted this here
Да, так и есть. Я обычно пишу x / 4, а не x >> 2, т.к. всё же деление воспринимается более интуитивно, а компилятор сам решит как поступить. Просто о том, что значит / 4, мой мозг узнал в 1 классе, а про >> 2 — лет 6 назад, так что ему всё же быстрее понять / 4 — не выходит из кэша, а за >> 2 надо лезть в оперативку :)
ещё по поводу ошибок:
int b[L];
палитру лучше было сделать из (unsigned char)
lAB += (int)(valueR * 0.299 + valueG * 0.587 + valueB * 0.114);
при округлении в этом месте будет теряться много информации, например, если весь буфер будет заполнен синим цветом с яркостью 6, то переменная lAB, которая средняя яркость всех пикселей, будет равна 0.
ну и ещё много мелочей, вроде «return 0;» в функции которая возвращает void, кучи лишних приведений типов и ни о чём не говорящих названий переменных.
(у поста уже 17 плюсов, кто все эти люди? :) )
int b[L];
палитру лучше было сделать из (unsigned char)
lAB += (int)(valueR * 0.299 + valueG * 0.587 + valueB * 0.114);
при округлении в этом месте будет теряться много информации, например, если весь буфер будет заполнен синим цветом с яркостью 6, то переменная lAB, которая средняя яркость всех пикселей, будет равна 0.
ну и ещё много мелочей, вроде «return 0;» в функции которая возвращает void, кучи лишних приведений типов и ни о чём не говорящих названий переменных.
(у поста уже 17 плюсов, кто все эти люди? :) )
Алгоритм распаралелен неправильно, но проблема здесь не с забиванием шины памяти. Даже на Sandy Bridge в идеально оптимизированном коде на одном потоке можно получить максимум 75% пропускной способности. А невекторизованным кодом забить шину памяти ещё сложнее. Проблема с кодом у автора поста в том, что разные потоки пишут в одни и те же строки кэша. Когда один из потоков хочет записать что-то в память, он проверяет, не закэширована ли эта строка в кэше другого процессора. Т.к. все потоки пишут в одни и те же строки кэша, скорее всего обнаружится, что эту строку кэша уже изменил другой поток. В результате будет отправлен запрос RFO (Request for ownership) к другому ядру процессора, чтобы оно прислало изменённую строку кэша. Попутно контроллер памяти отследит RFO запрос, и запишет изменённую строку кэша обратно в память (это нужно чтобы правильно работал MESI алгоритм). Всё это, естественно, очень не быстро, и главное, превращает код в по сути однопоточный.
еще одна очевидная оптимизация первого цикла. +5% буст
unsigned int midBright = 0, midBright1 = 0, midBright2 = 0;
for (size_t i = 0; i < dataSize; )
{
midBright += imageData[i++];
midBright1 += imageData[i++];
midBright2 += imageData[i++];
}
midBright = (midBright * 77 + midBright1 * 150 + midBright2 * 29) / (256 * dataSize / 3);
кстати можно даже вернуть вычисления с двойной точностью без потерь производительности
Только сначала лучше померить, действительно ли это скомпилируется в более быстрый код. У меня было несколько идей по поводу второго цикла: пробовал производить вычисления вместо обращения к палитре, пробовал считать по 4 байта и писать сразу dword, пробовал разносить чтение и запись на 1024 байта, пробовал вручную разворачивать цикл, в итоге оставил самый простой вариант, как самый быстрый.
кстати в имеющемся виде 255 максимальное значение субпикселя. в кадре fullHD их 2млн штук каждого цвета. итого после финального умножения 32хбитный инт переполняется.
то бишь надо либо сначала делить, а потом умножать, но так погрешность выше.
либо long int надо брать, ну либо возвращаться к double, как у автора.
то бишь надо либо сначала делить, а потом умножать, но так погрешность выше.
либо long int надо брать, ну либо возвращаться к double, как у автора.
А да, не заметил сразу, что умножения вынесены за цикл. Да, так будет быстрее.
Данный алгоритм не учитывает нелинейность исходного цветового пространства. Так делать нельзя.
Sign up to leave a comment.
Реализация RGB-алгоритма изменения контраста изображения