Re: [PATCH 3/3] debug.h: hint a compiler: TRACE is not executed in common usage
On 29.01.2019 01:17, Marvin wrote:
Thank you for your contribution to Wine!
This is an automated notification to let you know that your patch has been reviewed and its status set to "Rejected".
This means that the patch has been rejected by a reviewer. You should have received a mail explaining why it was rejected. You need to fix the issue and resend the patch, or if you are convinced that your patch is good as is, you should reply to the rejection message with your counterarguments.
If you do not understand the reason for this status, disagree with our assessment, or are simply not sure how to proceed next, please ask for clarification by replying to this email.
Hi, I don't understand, why the 3-rd patch was marked rejected? AFAIK it's being discussed.
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
On 29.01.2019 01:17, Marvin wrote:
Thank you for your contribution to Wine!
This is an automated notification to let you know that your patch has been reviewed and its status set to "Rejected".
This means that the patch has been rejected by a reviewer. You should have received a mail explaining why it was rejected. You need to fix the issue and resend the patch, or if you are convinced that your patch is good as is, you should reply to the rejection message with your counterarguments.
If you do not understand the reason for this status, disagree with our assessment, or are simply not sure how to proceed next, please ask for clarification by replying to this email.
Hi, I don't understand, why the 3-rd patch was marked rejected? AFAIK it's being discussed.
I explained that such micro-optimizations won't be accepted, unless there is clear evidence that they make a difference. Your numbers are not convincing enough I'm afraid. -- Alexandre Julliard julliard(a)winehq.org
On 29.01.2019 11:01, Alexandre Julliard wrote:
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
On 29.01.2019 01:17, Marvin wrote:
Thank you for your contribution to Wine!
This is an automated notification to let you know that your patch has been reviewed and its status set to "Rejected".
This means that the patch has been rejected by a reviewer. You should have received a mail explaining why it was rejected. You need to fix the issue and resend the patch, or if you are convinced that your patch is good as is, you should reply to the rejection message with your counterarguments.
If you do not understand the reason for this status, disagree with our assessment, or are simply not sure how to proceed next, please ask for clarification by replying to this email.
Hi, I don't understand, why the 3-rd patch was marked rejected? AFAIK it's being discussed.
I explained that such micro-optimizations won't be accepted, unless there is clear evidence that they make a difference. Your numbers are not convincing enough I'm afraid.
But why? It's still an optimization, and as being a burden it is very tiny. It is definitely smaller than the burden that my rejected refactoring was removing, why can't it be accepted? Okay, if we're into this discussion: can you please formalize, what burden does it add from your point of view?
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
On 29.01.2019 11:01, Alexandre Julliard wrote:
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
On 29.01.2019 01:17, Marvin wrote:
Thank you for your contribution to Wine!
This is an automated notification to let you know that your patch has been reviewed and its status set to "Rejected".
This means that the patch has been rejected by a reviewer. You should have received a mail explaining why it was rejected. You need to fix the issue and resend the patch, or if you are convinced that your patch is good as is, you should reply to the rejection message with your counterarguments.
If you do not understand the reason for this status, disagree with our assessment, or are simply not sure how to proceed next, please ask for clarification by replying to this email.
Hi, I don't understand, why the 3-rd patch was marked rejected? AFAIK it's being discussed.
I explained that such micro-optimizations won't be accepted, unless there is clear evidence that they make a difference. Your numbers are not convincing enough I'm afraid.
But why? It's still an optimization, and as being a burden it is very tiny. It is definitely smaller than the burden that my rejected refactoring was removing, why can't it be accepted?
Okay, if we're into this discussion: can you please formalize, what burden does it add from your point of view?
The default position is to keep the code as is; if you want to change it, it's up to you to demonstrate that the change is worth it. That's true for every submitted patch; it's even more true when making changes to things like debug.h that are used everywhere, where even a small change can have large consequences. -- Alexandre Julliard julliard(a)winehq.org
On 29.01.2019 12:08, Alexandre Julliard wrote:
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
On 29.01.2019 11:01, Alexandre Julliard wrote:
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
On 29.01.2019 01:17, Marvin wrote:
Thank you for your contribution to Wine!
This is an automated notification to let you know that your patch has been reviewed and its status set to "Rejected".
This means that the patch has been rejected by a reviewer. You should have received a mail explaining why it was rejected. You need to fix the issue and resend the patch, or if you are convinced that your patch is good as is, you should reply to the rejection message with your counterarguments.
If you do not understand the reason for this status, disagree with our assessment, or are simply not sure how to proceed next, please ask for clarification by replying to this email.
Hi, I don't understand, why the 3-rd patch was marked rejected? AFAIK it's being discussed.
I explained that such micro-optimizations won't be accepted, unless there is clear evidence that they make a difference. Your numbers are not convincing enough I'm afraid.
But why? It's still an optimization, and as being a burden it is very tiny. It is definitely smaller than the burden that my rejected refactoring was removing, why can't it be accepted?
Okay, if we're into this discussion: can you please formalize, what burden does it add from your point of view?
The default position is to keep the code as is; if you want to change it, it's up to you to demonstrate that the change is worth it. That's true for every submitted patch; it's even more true when making changes to things like debug.h that are used everywhere, where even a small change can have large consequences.
Fair enough. Then, let's break down pros and cons for the patch mentioned in discussion: pros: 1. it improves general execution of the code. TRACE is the *most* often evaluated branch in wine code, because it is used everywhere in the codebase. Benchmark did show a small improvement, but even without it it's a clear and easy to achieve optimization. 2. it adds a macro that can be used for further optimization of performance critical parts of the code, such as various generic algorithms, heap scheduling, and what not. cons: 1. it adds a small burden The abstract "small burden" is added by any code. You can reject virtually any patch that increases a line count by stating that it adds a small code burden. In this particular case the "burden" resides in a part of codebase that rarely being looked upon, in addition it's just a line being wrapped into "__unlikely" which is hardly would take an effort for anyone to read. Especially if I add the "unlikely" declaration just above the place where it's used. Codeweaver's customers would surely appreciate if their software would work a bit faster. In particular on those Macs with slow CPUs, my gf has one. And then I heard, Codeweavers are trying to extend their market to Android, right? Which has even slower CPUs. Let's just let this improvement in, please.
On 1/29/19 10:01 AM, Alexandre Julliard wrote:
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
On 29.01.2019 01:17, Marvin wrote:
Thank you for your contribution to Wine!
This is an automated notification to let you know that your patch has been reviewed and its status set to "Rejected".
This means that the patch has been rejected by a reviewer. You should have received a mail explaining why it was rejected. You need to fix the issue and resend the patch, or if you are convinced that your patch is good as is, you should reply to the rejection message with your counterarguments.
If you do not understand the reason for this status, disagree with our assessment, or are simply not sure how to proceed next, please ask for clarification by replying to this email.
Hi, I don't understand, why the 3-rd patch was marked rejected? AFAIK it's being discussed.
I explained that such micro-optimizations won't be accepted, unless there is clear evidence that they make a difference. Your numbers are not convincing enough I'm afraid.
FWIW, just a side note, speaking in general. Most micro-optimizations have small benefits, just as they have a tiny burden individually. As the burden increases by applying more of them, so does the benefit as it piles up, so it's not all that bad and works both ways, IMO. I guess in this case, for potential users who want to get max performance out of it, disabling TRACE and compiling wine themselves should be better. (can always just keep a normal compiled Wine along to TRACE problems and post bug reports and so on, if they run into any)
On 29.01.2019 14:22, Gabriel Ivăncescu wrote:
On 1/29/19 10:01 AM, Alexandre Julliard wrote:
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
On 29.01.2019 01:17, Marvin wrote:
Thank you for your contribution to Wine!
This is an automated notification to let you know that your patch has been reviewed and its status set to "Rejected".
This means that the patch has been rejected by a reviewer. You should have received a mail explaining why it was rejected. You need to fix the issue and resend the patch, or if you are convinced that your patch is good as is, you should reply to the rejection message with your counterarguments.
If you do not understand the reason for this status, disagree with our assessment, or are simply not sure how to proceed next, please ask for clarification by replying to this email.
Hi, I don't understand, why the 3-rd patch was marked rejected? AFAIK it's being discussed.
I explained that such micro-optimizations won't be accepted, unless there is clear evidence that they make a difference. Your numbers are not convincing enough I'm afraid.
FWIW, just a side note, speaking in general. Most micro-optimizations have small benefits, just as they have a tiny burden individually. As the burden increases by applying more of them, so does the benefit as it piles up, so it's not all that bad and works both ways, IMO.
I guess in this case, for potential users who want to get max performance out of it, disabling TRACE and compiling wine themselves should be better. (can always just keep a normal compiled Wine along to TRACE problems and post bug reports and so on, if they run into any)
I disagree that less branch-misses is a tiny improvement. I've read various posts where a particular algorithm become 10-15% faster by improving branch-prediction. I can't remember links ATM, but from a quick search I see e.g this page http://people.csail.mit.edu/jaffer/CNS/interpreter-branch which says they get 10% speed improvement due to branch prediction. It's easy to imagine that such a TRACE inserted in the middle of some algo in wine may reduce its execution speed.
On 1/29/19 1:33 PM, Konstantin Kharlamov wrote:
On 29.01.2019 14:22, Gabriel Ivăncescu wrote:
On 1/29/19 10:01 AM, Alexandre Julliard wrote:
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
On 29.01.2019 01:17, Marvin wrote:
Thank you for your contribution to Wine!
This is an automated notification to let you know that your patch has been reviewed and its status set to "Rejected".
This means that the patch has been rejected by a reviewer. You should have received a mail explaining why it was rejected. You need to fix the issue and resend the patch, or if you are convinced that your patch is good as is, you should reply to the rejection message with your counterarguments.
If you do not understand the reason for this status, disagree with our assessment, or are simply not sure how to proceed next, please ask for clarification by replying to this email.
Hi, I don't understand, why the 3-rd patch was marked rejected? AFAIK it's being discussed.
I explained that such micro-optimizations won't be accepted, unless there is clear evidence that they make a difference. Your numbers are not convincing enough I'm afraid.
FWIW, just a side note, speaking in general. Most micro-optimizations have small benefits, just as they have a tiny burden individually. As the burden increases by applying more of them, so does the benefit as it piles up, so it's not all that bad and works both ways, IMO.
I guess in this case, for potential users who want to get max performance out of it, disabling TRACE and compiling wine themselves should be better. (can always just keep a normal compiled Wine along to TRACE problems and post bug reports and so on, if they run into any)
I disagree that less branch-misses is a tiny improvement. I've read various posts where a particular algorithm become 10-15% faster by improving branch-prediction.
I can't remember links ATM, but from a quick search I see e.g this page http://people.csail.mit.edu/jaffer/CNS/interpreter-branch which says they get 10% speed improvement due to branch prediction.
It's easy to imagine that such a TRACE inserted in the middle of some algo in wine may reduce its execution speed.
Yeah, I was speaking in general. Even if micro-optimizations have small benefits individually (in general), it piles up when more of them get added. Of course, so does the burden, in some cases. But my point was that it's not *only* the burden which piles up, so it's not all negative. ;-)
On 1/29/19 5:33 AM, Konstantin Kharlamov wrote:
On 29.01.2019 14:22, Gabriel Ivăncescu wrote:
On 1/29/19 10:01 AM, Alexandre Julliard wrote:
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
On 29.01.2019 01:17, Marvin wrote:
Thank you for your contribution to Wine!
This is an automated notification to let you know that your patch has been reviewed and its status set to "Rejected".
This means that the patch has been rejected by a reviewer. You should have received a mail explaining why it was rejected. You need to fix the issue and resend the patch, or if you are convinced that your patch is good as is, you should reply to the rejection message with your counterarguments.
If you do not understand the reason for this status, disagree with our assessment, or are simply not sure how to proceed next, please ask for clarification by replying to this email.
Hi, I don't understand, why the 3-rd patch was marked rejected? AFAIK it's being discussed.
I explained that such micro-optimizations won't be accepted, unless there is clear evidence that they make a difference. Your numbers are not convincing enough I'm afraid.
FWIW, just a side note, speaking in general. Most micro-optimizations have small benefits, just as they have a tiny burden individually. As the burden increases by applying more of them, so does the benefit as it piles up, so it's not all that bad and works both ways, IMO.
I guess in this case, for potential users who want to get max performance out of it, disabling TRACE and compiling wine themselves should be better. (can always just keep a normal compiled Wine along to TRACE problems and post bug reports and so on, if they run into any)
I disagree that less branch-misses is a tiny improvement. I've read various posts where a particular algorithm become 10-15% faster by improving branch-prediction.
I can't remember links ATM, but from a quick search I see e.g this page http://people.csail.mit.edu/jaffer/CNS/interpreter-branch which says they get 10% speed improvement due to branch prediction.
It's easy to imagine that such a TRACE inserted in the middle of some algo in wine may reduce its execution speed.
If we're arguing that less branch mispredictions makes an algorithm faster, can't we cut out the middleman and measure speed improvement directly? If there really is an optimization there, can we run perhaps more than 4 tests to prove it, especially if the results have such wide variance?
On 29.01.2019 18:05, Zebediah Figura wrote:
On 1/29/19 5:33 AM, Konstantin Kharlamov wrote:
On 29.01.2019 14:22, Gabriel Ivăncescu wrote:
On 1/29/19 10:01 AM, Alexandre Julliard wrote:
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
On 29.01.2019 01:17, Marvin wrote:
Thank you for your contribution to Wine!
This is an automated notification to let you know that your patch has been reviewed and its status set to "Rejected".
This means that the patch has been rejected by a reviewer. You should have received a mail explaining why it was rejected. You need to fix the issue and resend the patch, or if you are convinced that your patch is good as is, you should reply to the rejection message with your counterarguments.
If you do not understand the reason for this status, disagree with our assessment, or are simply not sure how to proceed next, please ask for clarification by replying to this email.
Hi, I don't understand, why the 3-rd patch was marked rejected? AFAIK it's being discussed.
I explained that such micro-optimizations won't be accepted, unless there is clear evidence that they make a difference. Your numbers are not convincing enough I'm afraid.
FWIW, just a side note, speaking in general. Most micro-optimizations have small benefits, just as they have a tiny burden individually. As the burden increases by applying more of them, so does the benefit as it piles up, so it's not all that bad and works both ways, IMO.
I guess in this case, for potential users who want to get max performance out of it, disabling TRACE and compiling wine themselves should be better. (can always just keep a normal compiled Wine along to TRACE problems and post bug reports and so on, if they run into any)
I disagree that less branch-misses is a tiny improvement. I've read various posts where a particular algorithm become 10-15% faster by improving branch-prediction.
I can't remember links ATM, but from a quick search I see e.g this page http://people.csail.mit.edu/jaffer/CNS/interpreter-branch which says they get 10% speed improvement due to branch prediction.
It's easy to imagine that such a TRACE inserted in the middle of some algo in wine may reduce its execution speed.
If we're arguing that less branch mispredictions makes an algorithm faster, can't we cut out the middleman and measure speed improvement directly?
If there really is an optimization there, can we run perhaps more than 4 tests to prove it, especially if the results have such wide variance?
Sure, thanks for reply, will do. To give some update: measuring with Xonotic benchmark didn't show anything beyond noise, but then it's likely GPU-bound, I gotta try some thread-spawning or memory allocation benchmarks. I tried a couple of random benchmarks from web, but they didn't work. I figured phoronix-test-suite can be used, but it downloads x86_64 libs, whilst my wine build for testing is purely 32-bit. So, I'll get to re-building everything tomorrow, will report back then.
On 29.01.2019 22:59, Konstantin Kharlamov wrote:
On 29.01.2019 18:05, Zebediah Figura wrote:
On 1/29/19 5:33 AM, Konstantin Kharlamov wrote:
On 29.01.2019 14:22, Gabriel Ivăncescu wrote:
On 1/29/19 10:01 AM, Alexandre Julliard wrote:
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
On 29.01.2019 01:17, Marvin wrote: > Thank you for your contribution to Wine! > > This is an automated notification to let you know that your patch has > been reviewed and its status set to "Rejected". > > This means that the patch has been rejected by a reviewer. You should > have received a mail explaining why it was rejected. You need to fix > the issue and resend the patch, or if you are convinced that your > patch is good as is, you should reply to the rejection message with > your counterarguments. > > If you do not understand the reason for this status, disagree with our > assessment, or are simply not sure how to proceed next, please ask for > clarification by replying to this email.
Hi, I don't understand, why the 3-rd patch was marked rejected? AFAIK it's being discussed.
I explained that such micro-optimizations won't be accepted, unless there is clear evidence that they make a difference. Your numbers are not convincing enough I'm afraid.
FWIW, just a side note, speaking in general. Most micro-optimizations have small benefits, just as they have a tiny burden individually. As the burden increases by applying more of them, so does the benefit as it piles up, so it's not all that bad and works both ways, IMO.
I guess in this case, for potential users who want to get max performance out of it, disabling TRACE and compiling wine themselves should be better. (can always just keep a normal compiled Wine along to TRACE problems and post bug reports and so on, if they run into any)
I disagree that less branch-misses is a tiny improvement. I've read various posts where a particular algorithm become 10-15% faster by improving branch-prediction.
I can't remember links ATM, but from a quick search I see e.g this page http://people.csail.mit.edu/jaffer/CNS/interpreter-branch which says they get 10% speed improvement due to branch prediction.
It's easy to imagine that such a TRACE inserted in the middle of some algo in wine may reduce its execution speed.
If we're arguing that less branch mispredictions makes an algorithm faster, can't we cut out the middleman and measure speed improvement directly?
If there really is an optimization there, can we run perhaps more than 4 tests to prove it, especially if the results have such wide variance?
Sure, thanks for reply, will do. To give some update: measuring with Xonotic benchmark didn't show anything beyond noise, but then it's likely GPU-bound, I gotta try some thread-spawning or memory allocation benchmarks. I tried a couple of random benchmarks from web, but they didn't work. I figured phoronix-test-suite can be used, but it downloads x86_64 libs, whilst my wine build for testing is purely 32-bit.
So, I'll get to re-building everything tomorrow, will report back then.
Unfortunately phoronix-test-suite doesn't run under wine, it quits with "can't recognize CV_redist.x64.xe' as an internal or external command". I'll test later if it works under Windows and report a wine bug if it does; but if anybody is aware of any other free windows benchmarkss for memory allocation of threads creation that work under wine, I'll be happy to try them out. I guess there's no haste since someone for some reason told Marvin to reject my patch (let's hope it was just a misclick without any malicious intention).
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
I'll test later if it works under Windows and report a wine bug if it does; but if anybody is aware of any other free windows benchmarkss for memory allocation of threads creation that work under wine, I'll be happy to try them out.
I guess there's no haste since someone for some reason told Marvin to reject my patch (let's hope it was just a misclick without any malicious intention).
I rejected the patch, and I explained why. If you can come up with convincing benchmarks numbers, please resend it and we can discuss it further. -- Alexandre Julliard julliard(a)winehq.org
On 30.01.2019 13:43, Alexandre Julliard wrote:
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
I'll test later if it works under Windows and report a wine bug if it does; but if anybody is aware of any other free windows benchmarkss for memory allocation of threads creation that work under wine, I'll be happy to try them out.
I guess there's no haste since someone for some reason told Marvin to reject my patch (let's hope it was just a misclick without any malicious intention).
I rejected the patch, and I explained why. If you can come up with convincing benchmarks numbers, please resend it and we can discuss it further.
We're programmers, please, let's stay technical. Your explanation was some abstract "burden", and I asked you to formalize the "burden". You ignored me. At the same time on my part I provided you with good arguments why the patch is nice to have. What you did next is ignoring my reply whatsoever and rejecting the patch. And I must mention, you did that in fact twice, even though discussion was ongoing. Even *if you are correct*, your actions so far were based solely off a personal opinion, and not off any technical arguments. This is not a good development practice, please bear this in mind in the future. ---------- In other news, I managed to build "osbench" for Windows, and in thread creation/memory allocation benches I didn't see improvement beyond statistical noise, so let's hold off this patch until we find a workload that definitely gets improved it.
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
On 30.01.2019 13:43, Alexandre Julliard wrote:
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
I'll test later if it works under Windows and report a wine bug if it does; but if anybody is aware of any other free windows benchmarkss for memory allocation of threads creation that work under wine, I'll be happy to try them out.
I guess there's no haste since someone for some reason told Marvin to reject my patch (let's hope it was just a misclick without any malicious intention).
I rejected the patch, and I explained why. If you can come up with convincing benchmarks numbers, please resend it and we can discuss it further.
We're programmers, please, let's stay technical. Your explanation was some abstract "burden", and I asked you to formalize the "burden". You ignored me. At the same time on my part I provided you with good arguments why the patch is nice to have. What you did next is ignoring my reply whatsoever and rejecting the patch. And I must mention, you did that in fact twice, even though discussion was ongoing.
You are the only one who's "discussing" it. I asked you to show convincing numbers, so you have a path forward. Until you come back with the numbers, there's no reason to continue the argument. The most precious resource we need to conserve is not branch prediction cycles, it's developers' cycles. That's why you shouldn't change things that don't need changing, you shouldn't add complexity that isn't needed, and you should spend extra effort on your side to save other developers' time. These are the kind of burdens I'm talking about.
In other news, I managed to build "osbench" for Windows, and in thread creation/memory allocation benches I didn't see improvement beyond statistical noise, so let's hold off this patch until we find a workload that definitely gets improved it.
Glad to see we agree ;-) -- Alexandre Julliard julliard(a)winehq.org
On 30.01.2019 20:13, Alexandre Julliard wrote:
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
On 30.01.2019 13:43, Alexandre Julliard wrote:
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
I'll test later if it works under Windows and report a wine bug if it does; but if anybody is aware of any other free windows benchmarkss for memory allocation of threads creation that work under wine, I'll be happy to try them out.
I guess there's no haste since someone for some reason told Marvin to reject my patch (let's hope it was just a misclick without any malicious intention).
I rejected the patch, and I explained why. If you can come up with convincing benchmarks numbers, please resend it and we can discuss it further.
We're programmers, please, let's stay technical. Your explanation was some abstract "burden", and I asked you to formalize the "burden". You ignored me. At the same time on my part I provided you with good arguments why the patch is nice to have. What you did next is ignoring my reply whatsoever and rejecting the patch. And I must mention, you did that in fact twice, even though discussion was ongoing.
You are the only one who's "discussing" it. I asked you to show convincing numbers, so you have a path forward. Until you come back with the numbers, there's no reason to continue the argument.
You see, this is exactly where you take your own opinion above anything else. There *are* other (mentioned) reasons for including the patch beside "convincing numbers", but you silently decided for everyone else on the list.
The most precious resource we need to conserve is not branch prediction cycles, it's developers' cycles. That's why you shouldn't change things that don't need changing, you shouldn't add complexity that isn't needed
Well, I guessed that by burden you mean code complexity. And I already replied on that one, at least in the mail with pros'n'cons. But you didn't reply, which made me think you might have meant something else on your mind. Discussion doesn't work when you expose your disagreement by a blunt force, such as silently rejecting patches.
, and you should spend extra effort on your side to save other developers' time. These are the kind of burdens I'm talking about.
This advice works both sides. If you took time to prove your point instead of what you did, that likely wouldn't have resulted in a dozen of mails, ⅔ of which doesn't move discussion anywhere.
On Wed, 30 Jan 2019 at 21:06, Konstantin Kharlamov <hi-angel(a)yandex.ru> wrote:
On 30.01.2019 20:13, Alexandre Julliard wrote:
You are the only one who's "discussing" it. I asked you to show convincing numbers, so you have a path forward. Until you come back with the numbers, there's no reason to continue the argument.
You see, this is exactly where you take your own opinion above anything else. There *are* other (mentioned) reasons for including the patch beside "convincing numbers", but you silently decided for everyone else on the list.
As someone else on the list, that's not quite how I see this conversation. Even if it *were* though, saying "no" is pretty much a maintainer's prerogative.
The most precious resource we need to conserve is not branch prediction cycles, it's developers' cycles. That's why you shouldn't change things that don't need changing, you shouldn't add complexity that isn't needed
Well, I guessed that by burden you mean code complexity. And I already replied on that one, at least in the mail with pros'n'cons. But you didn't reply, which made me think you might have meant something else on your mind. Discussion doesn't work when you expose your disagreement by a blunt force, such as silently rejecting patches.
I'd hardly call this thread silent. If I can try to give a constructive suggestion though, I get the impression that you put a lot of value on the patch status as displayed on the status page. That status isn't that important; the status page is meant to give people more insight into different reasons a patch might not have been committed, and avoid losing track of patches. Your path forward is still to try to make a more convincing argument, regardless of what the status page may say.
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
Well, I guessed that by burden you mean code complexity. And I already replied on that one, at least in the mail with pros'n'cons. But you didn't reply, which made me think you might have meant something else on your mind. Discussion doesn't work when you expose your disagreement by a blunt force, such as silently rejecting patches.
Silently??? I believe that I've asked you at least 6 times so far to provide more convincing numbers. You are free to disagree with my judgment, but you can't claim that you don't know the reason for the rejection. -- Alexandre Julliard julliard(a)winehq.org
Thanks for answers, let's just close the topic. There have been, sort of, agreement; and there were enough mails that I hope we understand each other. At this point, I think, further discussion would only be a waste of time and energy for everyone involved. On 30.01.2019 21:12, Alexandre Julliard wrote:
Konstantin Kharlamov <hi-angel(a)yandex.ru> writes:
Well, I guessed that by burden you mean code complexity. And I already replied on that one, at least in the mail with pros'n'cons. But you didn't reply, which made me think you might have meant something else on your mind. Discussion doesn't work when you expose your disagreement by a blunt force, such as silently rejecting patches.
Silently??? I believe that I've asked you at least 6 times so far to provide more convincing numbers. You are free to disagree with my judgment, but you can't claim that you don't know the reason for the rejection.
Am 30.01.19 um 09:51 schrieb Konstantin Kharlamov:
Unfortunately phoronix-test-suite doesn't run under wine, it quits with "can't recognize CV_redist.x64.xe' as an internal or external command". PTS has a feature where it can run benchmarks in Wine. Grep the source code and documentation for USE_WINE . You enable that by setting an environment variable.
See also https://openbenchmarking.org/tests/moihack . Those are some game benchmarks written with Wine in mind. Even if you cannot find a performance improvement from your idea now, keep an eye on it, as this may change in the future. It might be that games / apps / whatever are held back by something else, like unnecessary GPU<->CPU synchronization or wineserver calls, and once those other issues are fixed your idea has a bigger impact.
Am 30.01.19 um 09:51 schrieb Konstantin Kharlamov:
Unfortunately phoronix-test-suite doesn't run under wine, it quits with "can't recognize CV_redist.x64.xe' as an internal or external command". PTS has a feature where it can run benchmarks in Wine. Grep the source code and documentation for USE_WINE . You enable that by setting an environment variable.
See also https://openbenchmarking.org/tests/moihack . Those are some game benchmarks written with Wine in mind. Even if you cannot find a performance improvement from your idea now, keep an eye on it, as this may change in the future. It might be that games / apps / whatever are held back by something else, like unnecessary GPU<->CPU synchronization or wineserver calls, and once those other issues are fixed your idea has a bigger impact.
On 30.01.2019 15:08, Stefan Dösinger wrote:
Am 30.01.19 um 09:51 schrieb Konstantin Kharlamov:
Unfortunately phoronix-test-suite doesn't run under wine, it quits with "can't recognize CV_redist.x64.xe' as an internal or external command". PTS has a feature where it can run benchmarks in Wine. Grep the source code and documentation for USE_WINE . You enable that by setting an environment variable.
See also https://openbenchmarking.org/tests/moihack . Those are some game benchmarks written with Wine in mind.
Even if you cannot find a performance improvement from your idea now, keep an eye on it, as this may change in the future. It might be that games / apps / whatever are held back by something else, like unnecessary GPU<->CPU synchronization or wineserver calls, and once those other issues are fixed your idea has a bigger impact.
Oh, thanks, good to know! And I'll keep an eye on it as well as on other possible bottlenecks. I also wanted to say: thanks to everyone involved!
participants (6)
-
Alexandre Julliard -
Gabriel Ivăncescu -
Henri Verbeet -
Konstantin Kharlamov -
Stefan Dösinger -
Zebediah Figura