[PATCH v2 1/3] debug.h: cleanup TRACE macros

[Alexandre Julliard]

Konstantin Kharlamov <hi-angel at yandex.ru> writes:

> On 28.01.2019 19:04, Alexandre Julliard wrote:
>> Konstantin Kharlamov <hi-angel at yandex.ru> writes:
>>> While on it, could you please advice on the naming in the 3-rd patch?
>>> Initially I've used "unlikely" because that's how it's used in
>>> Mesa. Later then I saw in wine lots of lower-case macros with __
>>> prefix, so I changed it to "__unlikely". But then there are generic
>>> macros like "TRACE", and I think of renaming it to "UNLIKELY", which
>>> is 2 letters shorter, and is upper-case like yet another part of
>>> macros.
>>
>> Considering that the benchmarks don't show an improvement, I don't think
>> it's worth the trouble.
>
> Wait, how so if it does? Besides, even without the benchmark it's
> obvious that the TRACE appear in so many places that there inevitably
> would be branch-misses.
>
> In addition, this macro can be used for further optimization of
> performance critical parts of wine code.
>
> And what you mean by "the trouble" — it's just a few lines of code
> that doesn't add any maintainance burden.

That specific change is clearly a tiny burden, but it's the accumulation
of such micro-optimizations that ends up being a large burden. That's
why we require evidence that it actually helps.

By trying to measure it, you had the right approach, but you need to
accept the measurement results even if they are not what you
expected. If you can't demonstrate a clear improvement, then maybe it's
not as obviously a gain as it may seem at first glance.

If you want to optimize Wine, I'd suggest picking an app that runs
slower than it should, and figuring out where the bottlenecks are.
These are the places that are worth optimizing.

-- 
Alexandre Julliard
julliard at winehq.org