Matt Wilmas wrote:
Hi Dmitry,
----- Original Message -----
From: "Dmitry Stogov"
Sent: Monday, March 23, 2009
Hi Matt,
Matt Wilmas wrote:
Hi Dmitry,
While updating my version (more below), I noticed another problem
that I overlooked before with your leading zero check: "-0" is being
treated as numeric.
Yeah. Thank you for catching this. It should be fixed.
----- Original Message -----
From: "Dmitry Stogov"
Sent: Friday, March 20, 2009
Hi Matt,
I ran you version but it looked little bit slower (I checked it with
callgrind). So I kept the current CVS version. Anyway, it is not a
big problem to change it after RC.
Yeah, after I sent that message, I noticed other times were strange
(bench.php, etc.), so it looks like I was getting compiler weirdness
with that code layout... And of course looking at the code, there
shouldn't have been anything faster about it, though it was shorter,
which you confirmed with callgrind.
Anyway, my updated version seems good, still smaller, and should do
better with callgrind. :-) On 32-bit, the first digit of a 10 digit
number can be checked which avoids having "2x overflow" on
"5000000000", etc. that I pointed out with your original code. (No
problem on 64-bit.) Also made the terminating null check be last
again since it's least likely to fail...
http://realplain.com/php/handle_numeric.txt
OLD: http://realplain.com/php/handle_numeric-v1.txt
It looks like now your version must be better.
Are you sure that it can't miss overflow? Can it be proved?
Well, as far as I can tell, it can be proved. :-) On 32-bit, <= 9
digits is obviously OK, and if there's 10, it's only used if it starts
with a 1 or 2. Overflow with 2500000000 is caught by the </> 0 check,
and 5000000000, which the </> 0 check in your first version missed,
won't be used since we can tell from the first digit that it wouldn't
fit. On 64-bit, LONG_MAX starts with a 9, and there wouldn't be an
overflow missed by the </> 0 check until ULONG_MAX, which is another
digit longer, and would be skipped by the "numbers too long" check.
OK. I'll test and ally your patch after RC.
Dmitry.
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
