Replace temp with query
От: Аноним  
Дата: 06.03.07 10:50
Оценка:
Перечитываю тут на досуге "Рефакторинг" Фаулера.
Никак не могу понять (скорее, "принять") некоторые рефакторинги, напрямую связанные с ухудшением производительности.

Главный пример — subj.
Автор предлагает следующий фрагемент кода

double C::f()
{
    double tmp = a_ * b_;
    if (tmp > 10)
        return tmp * 0.5;
    else
        return tmp;
}


изменить на такой:

double C::ab()
{
    return a_ * b_;
}

double C::f()
{
    if (ab() > 10)
        return ab() * 0.5;
    else
        return ab();
}


Интересно ваше мнение по поводу того, действительно ли стоит жертвовать производительностью (и не кешировать вычисления в таких случаях в угоду более красивому коду) и положиться на профайлер? Честно говоря, у меня рука бы дрогнула такое написать, но я уже начинаю сомневаться

Спасибо!
Re: Replace temp with query
От: remark Россия http://www.1024cores.net/
Дата: 06.03.07 10:52
Оценка:
Здравствуйте, Аноним, Вы писали:

А>Перечитываю тут на досуге "Рефакторинг" Фаулера.

А>Никак не могу понять (скорее, "принять") некоторые рефакторинги, напрямую связанные с ухудшением производительности.

А в чём причина думать, что это будет работать с разной скоростью?


1024cores — all about multithreading, multicore, concurrency, parallelism, lock-free algorithms
Re: Replace temp with query
От: Sergey Россия  
Дата: 06.03.07 10:58
Оценка: -1
> Перечитываю тут на досуге "Рефакторинг" Фаулера.
> Никак не могу понять (скорее, "принять") некоторые рефакторинги, напрямую связанные с ухудшением производительности.
>
> Главный пример — subj.
> Автор предлагает следующий фрагемент кода
>
>
> double C::f()
> {
>    double tmp = a_ * b_;
>    if (tmp > 10)
>        return tmp * 0.5;
>    else
>        return tmp;
> }
>

>
> изменить на такой:
>
>
> double C::ab()
> {
>    return a_ * b_;
> }
> 
> double C::f()
> {
>    if (ab() > 10)
>        return ab() * 0.5;
>    else
>        return ab();
> }
>

>
> Интересно ваше мнение по поводу того, действительно ли стоит жертвовать производительностью (и не кешировать вычисления в таких случаях в угоду более красивому коду) и положиться на профайлер? Честно говоря, у меня рука бы дрогнула такое написать, но я уже начинаю сомневаться

Ну так как обычно — все зависит от конкретной ситуации. Когда-то можно пожертвовать производительностью, когда-то — нет.
А в приведенном примере одновременно ухудшается и читаемость, и производительность. Там разве что tmp как const double объявить имеет смысл, с точки зрения улучшения читаемости. Если уж совсем невмоготу — то поменять везде tmp на a_ * b_, а не на вызов функции.
Posted via RSDN NNTP Server 2.0
Одним из 33 полных кавалеров ордена "За заслуги перед Отечеством" является Геннадий Хазанов.
Re: Replace temp with query
От: sux Земля  
Дата: 06.03.07 11:06
Оценка:
Здравствуйте, Аноним, Вы писали:

А>Перечитываю тут на досуге "Рефакторинг" Фаулера.

А>Никак не могу понять (скорее, "принять") некоторые рефакторинги, напрямую связанные с ухудшением производительности.

А>Главный пример — subj.

А>Автор предлагает следующий фрагемент кода

А>
А>double C::f()
А>{
А>    double tmp = a_ * b_;
А>    if (tmp > 10)
А>        return tmp * 0.5;
А>    else
А>        return tmp;
А>}
А>


А>изменить на такой:


А>
А>double C::ab()
А>{
А>    return a_ * b_;
А>}

А>double C::f()
А>{
А>    if (ab() > 10)
А>        return ab() * 0.5;
А>    else
А>        return ab();
А>}
А>


А>Интересно ваше мнение по поводу того, действительно ли стоит жертвовать производительностью (и не кешировать вычисления в таких случаях в угоду более красивому коду) и положиться на профайлер? Честно говоря, у меня рука бы дрогнула такое написать, но я уже начинаю сомневаться


А>Спасибо!


поясните если неслжно неграмотному (нечитавшему еще Фаулера) смысл данного рефакторинга
Re: Replace temp with query
От: Какая разница Украина  
Дата: 06.03.07 11:06
Оценка:
Здравствуйте, Аноним, Вы писали:

А>Интересно ваше мнение по поводу того, действительно ли стоит жертвовать производительностью (и не кешировать вычисления в таких случаях в угоду более красивому коду) и положиться на профайлер? Честно говоря, у меня рука бы дрогнула такое написать, но я уже начинаю сомневаться


А>Спасибо!


Солидарен с тобой
"более красивый код" Фаулера фтопку
!0xDEAD
Re: Replace temp with query
От: Zigmar Израиль  
Дата: 06.03.07 11:23
Оценка: 1 (1)
Здравствуйте, Аноним, Вы писали:

А>Перечитываю тут на досуге "Рефакторинг" Фаулера.

А>Никак не могу понять (скорее, "принять") некоторые рефакторинги, напрямую связанные с ухудшением производительности.
Невнимательно перечитывал
Там вполне логично написано, что не стоит заниматься оптимизацией зарание, пока этого не нужно. А нужно это или нет, можно узнать, только отпрофайлив код, и чем код более структурированный — тем легче находить будет в нем находить баттлнеки. Так что рефактори по максимому, а потом попрофайлив, оптимизируй узкие места. И как почти всегда бывает, узкие места оказываются далеко не там, где кажется по началу. И баттлнеки, при хорошо структурированном коде, гораздо легче оптимизировать, так как код локализирован.
"To protect people you must slay people. To let people live you must let people die. This is the true teaching of the sword."
-Seijuro Hiko, "Rurouni Kensin"
Re[2]: Replace temp with query
От: Аноним  
Дата: 06.03.07 11:29
Оценка:
Здравствуйте, remark, Вы писали:

А>>Перечитываю тут на досуге "Рефакторинг" Фаулера.

А>>Никак не могу понять (скорее, "принять") некоторые рефакторинги, напрямую связанные с ухудшением производительности.

R>А в чём причина думать, что это будет работать с разной скоростью?


Очевидно в том, что ab() будет вызываться дважды.
Re[3]: Replace temp with query
От: remark Россия http://www.1024cores.net/
Дата: 06.03.07 11:36
Оценка:
Здравствуйте, Аноним, Вы писали:

А>Здравствуйте, remark, Вы писали:


А>>>Перечитываю тут на досуге "Рефакторинг" Фаулера.

А>>>Никак не могу понять (скорее, "принять") некоторые рефакторинги, напрямую связанные с ухудшением производительности.

R>>А в чём причина думать, что это будет работать с разной скоростью?


А>Очевидно в том, что ab() будет вызываться дважды.


Если ab() вызывается дважды с т.з. абстрактной с++ машины, это ещё не значит, что будет хоть какая-то разница по времени выполнения.


1024cores — all about multithreading, multicore, concurrency, parallelism, lock-free algorithms
Re: Replace temp with query
От: Шахтер Интернет  
Дата: 06.03.07 11:51
Оценка: +2
Здравствуйте, Аноним, Вы писали:

А>Перечитываю тут на досуге "Рефакторинг" Фаулера.

А>Никак не могу понять (скорее, "принять") некоторые рефакторинги, напрямую связанные с ухудшением производительности.

А>Главный пример — subj.

А>Автор предлагает следующий фрагемент кода

А>
А>double C::f()
А>{
А>    double tmp = a_ * b_;
А>    if (tmp > 10)
А>        return tmp * 0.5;
А>    else
А>        return tmp;
А>}
А>


А>изменить на такой:


А>
А>double C::ab()
А>{
А>    return a_ * b_;
А>}

А>double C::f()
А>{
А>    if (ab() > 10)
А>        return ab() * 0.5;
А>    else
А>        return ab();
А>}
А>


А>Интересно ваше мнение по поводу того, действительно ли стоит жертвовать производительностью (и не кешировать вычисления в таких случаях в угоду более красивому коду) и положиться на профайлер? Честно говоря, у меня рука бы дрогнула такое написать, но я уже начинаю сомневаться


А>Спасибо!


Вне контекста невозможно ответить на вопрос с уверенностью, но у меня второй пример вызывает сильные подозрения.
В первом случае мы "замораживаем" состояние объекта и вычисляем на его основе результат. Во втором -- вызывем метод ab() котрый даже не объявлен const и непонятно что он делает.
Лучше уж рефакторить так (если вообще это нужно в данном примере в чем я сильно сомневаюсь)


double C::Adjust(double prod) 
 {  
  if (prod > 10)
     return prod * 0.5;
  else
     return prod;
 }

double C::f()
{
 return Adjust(a_ * b_);
}
В XXI век с CCore.
Копай Нео, копай -- летать научишься. © Matrix. Парадоксы
Re[2]: Replace temp with query
От: Lazarenko Украина http://www.b2bits.com
Дата: 06.03.07 12:13
Оценка:
Здравствуйте, sux, Вы писали:

sux>поясните если неслжно неграмотному (нечитавшему еще Фаулера) смысл данного рефакторинга


Смысл в том, чтобы избавиться от локальных переменных, сделать код нагляднее и уменьшить вероятность ошибки.
With regards,
Vladislav Lazarenko
Re[2]: Replace temp with query
От: Аноним  
Дата: 06.03.07 14:12
Оценка:
Здравствуйте, Sergey, Вы писали:


S>Ну так как обычно — все зависит от конкретной ситуации. Когда-то можно пожертвовать производительностью, когда-то — нет.

S>А в приведенном примере одновременно ухудшается и читаемость, и производительность. Там разве что tmp как const double объявить имеет смысл, с точки зрения улучшения читаемости. Если уж совсем невмоготу — то поменять везде tmp на a_ * b_, а не на вызов функции.

Разницы в производительности тут скорее всего не будет, поскольку современные компилляторы с оптимизируют этот код. Скорее всего вызов функции будет заинлайнен.
-Владимир
Re: Replace temp with query
От: remark Россия http://www.1024cores.net/
Дата: 06.03.07 14:25
Оценка: 1 (1)
Здравствуйте, Аноним, Вы писали:

А>Перечитываю тут на досуге "Рефакторинг" Фаулера.

А>Никак не могу понять (скорее, "принять") некоторые рефакторинги, напрямую связанные с ухудшением производительности.

Ещё раз поясню свою мысль:

struct A
{
    double a_;
    double b_;

    double f()
    {
        double tmp = a_ * b_;
        if (tmp > 10)
            return tmp * 0.5;
        else
            return tmp;
    }
};

struct B
{
    double a_;
    double b_;

    double ab()
    {
        return a_ * b_;
    }

    double f()
    {
        if (ab() > 10)
            return ab() * 0.5;
        else
            return ab();
    }
};

int main()
{
    A a = {rand(), rand()};
    std::cout << a.f();

    B b = {rand(), rand()};
    std::cout << b.f();
}



Теперь самое интересное:

int main()
{
004014B0  sub         esp,14h 
    A a = {rand(), rand()};
004014B3  call        rand (405960h) 
004014B8  mov         dword ptr [esp],eax 
004014BB  fild        dword ptr [esp] 
004014BE  fstp        qword ptr [esp+4] 
004014C2  call        rand (405960h) 
004014C7  mov         dword ptr [esp],eax 
004014CA  fild        dword ptr [esp] 
    std::cout << a.f();
004014CD  fmul        qword ptr [esp+4] 
004014D1  fcom        qword ptr [__real@4024000000000000 (410128h)] 
004014D7  fnstsw      ax   
004014D9  test        ah,41h 
004014DC  jne         main+34h (4014E4h) 
004014DE  fmul        qword ptr [__real@3fe0000000000000 (410120h)] 
004014E4  sub         esp,8 
004014E7  mov         ecx,offset std::cout (4151F0h) 
004014EC  fstp        qword ptr [esp] 
004014EF  call        std::basic_ostream<char,std::char_traits<char> >::operator<< (401320h) 

    B b = {rand(), rand()};
004014F4  call        rand (405960h) 
004014F9  mov         dword ptr [esp],eax 
004014FC  fild        dword ptr [esp] 
004014FF  fstp        qword ptr [esp+4] 
00401503  call        rand (405960h) 
00401508  mov         dword ptr [esp],eax 
0040150B  fild        dword ptr [esp] 
    std::cout << b.f();
0040150E  fmul        qword ptr [esp+4] 
00401512  fcom        qword ptr [__real@4024000000000000 (410128h)] 
00401518  fnstsw      ax   
0040151A  test        ah,41h 
0040151D  jne         main+75h (401525h) 
0040151F  fmul        qword ptr [__real@3fe0000000000000 (410120h)] 
00401525  sub         esp,8 
00401528  mov         ecx,offset std::cout (4151F0h) 
0040152D  fstp        qword ptr [esp] 
00401530  call        std::basic_ostream<char,std::char_traits<char> >::operator<< (401320h) 
}
00401535  xor         eax,eax 
00401537  add         esp,14h 
0040153A  ret



Вывод: пишите как Вам удобнее/быстрее/хочется, покуда Вы записываете одно и то же поведение, нет никаких оснований полагать, что это будет выполняться медленнее или быстрее.


1024cores &mdash; all about multithreading, multicore, concurrency, parallelism, lock-free algorithms
Re[3]: Replace temp with query
От: Sergey Россия  
Дата: 06.03.07 16:15
Оценка:
> S>Ну так как обычно — все зависит от конкретной ситуации. Когда-то можно пожертвовать производительностью, когда-то — нет.
> S>А в приведенном примере одновременно ухудшается и читаемость, и производительность. Там разве что tmp как const double объявить имеет смысл, с точки зрения улучшения читаемости. Если уж совсем невмоготу — то поменять везде tmp на a_ * b_, а не на вызов функции.
>
> Разницы в производительности тут скорее всего не будет, поскольку современные компилляторы с оптимизируют этот код. Скорее всего вызов функции будет заинлайнен.
> -Владимир

Вообще-то в примере функция была определена вне тела класса. Поэтому я предположил, что и объявлена она не как инлайн. Соответственно, есть хорошие шансы на падение производительности.
Posted via RSDN NNTP Server 2.0
Одним из 33 полных кавалеров ордена "За заслуги перед Отечеством" является Геннадий Хазанов.
Re[2]: Replace temp with query
От: Roman Odaisky Украина  
Дата: 06.03.07 18:37
Оценка: :)
Здравствуйте, remark, Вы писали:

А>>Перечитываю тут на досуге "Рефакторинг" Фаулера.

А>>Никак не могу понять (скорее, "принять") некоторые рефакторинги, напрямую связанные с ухудшением производительности.

R>Ещё раз поясню свою мысль:


Так это ты — Фаулер?!

До последнего не верил в пирамиду Лебедева.
Re[4]: Replace temp with query
От: Ka3a4oK  
Дата: 06.03.07 20:44
Оценка:
На компиляторе с примитивным оптимизатором, я думаю, будет.
... << RSDN@Home 1.1.4 stable rev. 510>>
 
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.