On November 29, 2003 06:27 pm, Andrew de Quincey wrote:
I've replaced the calls to CharNextA()/CharNextW() with array increments and a local static function. This knocks 3.5 seconds off the file open dialogue in IDA.
Well, I'm not sure this is worth doing. First off, we're not fixing any app that makes use of CharNext{A,W}(). Second, why is your static method any faster than the real CharNextA()? Third, it's not correct to replace CharNextW() with an array increment. While it is true that currently our CharNextW() simply does the same thing, it should be fixed in the long run to properly deal with Unicode Surrogate Pairs: http://uk.geocities.com/BabelStone1357/Unicode/surrogates.html In other words, I don't think the patch is acceptable. What I'd suggest: -- figure out why the real CharNextA() is slow, and try to fix that -- figure out why it takes so long to call CharNextW() -- Dimi.
"Dimitrie O. Paun" <dpaun(a)rogers.com> writes:
Well, I'm not sure this is worth doing. First off, we're not fixing any app that makes use of CharNext{A,W}(). Second, why is your static method any faster than the real CharNextA()? Third, it's not correct to replace CharNextW() with an array increment. While it is true that currently our CharNextW() simply does the same thing, it should be fixed in the long run to properly deal with Unicode Surrogate Pairs:
While that's true in general, for the path functions it doesn't really matter since surrogates will never be path separators, so getting rid of CharNextW in that case is OK. CharNextA is another story however... -- Alexandre Julliard julliard(a)winehq.com
On Sunday 30 November 2003 01:23, Dimitrie O. Paun wrote:
On November 29, 2003 06:27 pm, Andrew de Quincey wrote:
I've replaced the calls to CharNextA()/CharNextW() with array increments and a local static function. This knocks 3.5 seconds off the file open dialogue in IDA.
Well, I'm not sure this is worth doing. First off, we're not fixing any app that makes use of CharNext{A,W}().
Er, it does. Datarescue IDA Pro. I regard the save box taking > 10 seconds to open as a bug. (Note: This fix only fixes 3.5 seconds of the problem. more work is needed)
Second, why is your static method any faster than the real CharNextA()? Third, it's not correct to replace CharNextW() with an array increment. While it is true that currently our CharNextW() simply does the same thing, it should be fixed in the long run to properly deal with Unicode Surrogate Pairs:
http://uk.geocities.com/BabelStone1357/Unicode/surrogates.html
In other words, I don't think the patch is acceptable. What I'd suggest: -- figure out why the real CharNextA() is slow, and try to fix that -- figure out why it takes so long to call CharNextW()
Its actually just the CharNextW function. The CharNextA thing I admit was premature optimisation that I have not tested and should be removed. As to why it takes so long to call CharNextW. Why indeed? Its a function defined in a DLL external to the shlwapi one. As there doesn't appear to be any funky code going on, it must be going through the normal WINE layers for calling such a function, surely?
In other words, I don't think the patch is acceptable. What I'd suggest: -- figure out why the real CharNextA() is slow, and try to fix that -- figure out why it takes so long to call CharNextW()
As I said originally, its not that CharNextW/CharNextA are particularly slow, its the number of times they're called. Obviously calling a function to do a simple string increment is going to be slower. CharNextW takes on average 168.5340us. However it is called 12396 times when opening the save box for IDA. As I said, IDA supplies quite a large string of filter expressions to the save box. This results in a total time of 2089146.9655us being spent in the CharNextW function. I will move CharNextW to be a static function of that file to see if its some weirdness with external functions in WINE, or if its just calling a function in general.
On Monday 01 December 2003 09:32, Andrew de Quincey wrote:
In other words, I don't think the patch is acceptable. What I'd suggest: -- figure out why the real CharNextA() is slow, and try to fix that -- figure out why it takes so long to call CharNextW()
As I said originally, its not that CharNextW/CharNextA are particularly slow, its the number of times they're called. Obviously calling a function to do a simple string increment is going to be slower.
CharNextW takes on average 168.5340us. However it is called 12396 times when opening the save box for IDA. As I said, IDA supplies quite a large string of filter expressions to the save box. This results in a total time of 2089146.9655us being spent in the CharNextW function.
I will move CharNextW to be a static function of that file to see if its some weirdness with external functions in WINE, or if its just calling a function in general.
Doh! You're right. I've just tried it with the original code again, and its much faster now. The save window is now taking only a couple of seconds (as opposed to >10!) to open. Something else must have changed on my system to make it faster... (perhaps fewer calls to CharNextW). Anyway, I'll keep going at it.
Andrew de Quincey <adq_dvb(a)lidskialf.net> writes:
CharNextW takes on average 168.5340us. However it is called 12396 times when opening the save box for IDA. As I said, IDA supplies quite a large string of filter expressions to the save box. This results in a total time of 2089146.9655us being spent in the CharNextW function.
That's not possible, such a simple function cannot take 168us, unless you have a 1Mhz CPU... How did you measure it? -- Alexandre Julliard julliard(a)winehq.com
On December 1, 2003 03:26 pm, Alexandre Julliard wrote:
That's not possible, such a simple function cannot take 168us, unless you have a 1Mhz CPU... How did you measure it?
I think he measured it across the call: start CharNextW end but it still doesn't add up. If his measurement is correct, we certainly have a high (it's an understatement, IIRC a syscall on Linux is on the order of a few us) call overhead. Maybe a bit of disassembly might help, but I can't imagine what could have gone wrong in such a trivial of a case. -- Dimi.
On Monday 01 December 2003 20:33, Dimitrie O. Paun wrote:
On December 1, 2003 03:26 pm, Alexandre Julliard wrote:
That's not possible, such a simple function cannot take 168us, unless you have a 1Mhz CPU... How did you measure it?
I think he measured it across the call:
start CharNextW end
but it still doesn't add up. If his measurement is correct, we certainly have a high (it's an understatement, IIRC a syscall on Linux is on the order of a few us) call overhead. Maybe a bit of disassembly might help, but I can't imagine what could have gone wrong in such a trivial of a case.
I'm really distrustful of those results; something very weird seemed to happen during that test. I will re-run it again and let you know.
participants (3)
-
Alexandre Julliard -
Andrew de Quincey -
Dimitrie O. Paun