On 28.01.2019 22:10, Alexandre Julliard wrote:
Konstantin Kharlamov hi-angel@yandex.ru writes:
On 28.01.2019 19:04, Alexandre Julliard wrote:
Konstantin Kharlamov hi-angel@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.
5 millions branch-misses less on average is a clear improvement. Besides you need to consider that given TRACE is used a lot in the code, there clearly can be workloads where TRACE is executed more often, and hence results in slower execution.
I also don't see why would it affect a burden increase in the future. If you're referring to using "unlikely" in more places in the code — I can assure you, it doesn't make it any less readable, you can see examples here https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/gallium/drivers/r60...