Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:43394
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain realplain.com from 209.151.69.1 cause and error)
Message-ID: <31133F79731047CB9918A6D54FB908EC@pc1>
To: <internals@lists.php.net>,
	"Dmitry Stogov" <dmitry@zend.com>
Cc: "Lukas Kahwe Smith" <mls@pooteeweet.org>,
	=?iso-8859-1?Q?Johannes_Schl=FCter?= <johannes@php.net>
References: <1113CE12226949C2939A31971420991F@pc1> <49C0A7C7.8000804@zend.com> <12E613FAA1C9422B948F00B61AD32366@pc1> <49C134E5.7020706@zend.com>
Date: Wed, 18 Mar 2009 13:35:32 -0500
MIME-Version: 1.0
Content-Type: text/plain;
	format=flowed;
	charset="iso-8859-1";
	reply-type=original
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH] Bug #45877: LONG_MAX/MIN array key as string/int
From: php_lists@realplain.com ("Matt Wilmas")

Hi Dmitry,

----- Original Message -----
From: "Dmitry Stogov"
Sent: Wednesday, March 18, 2009

> Hi Matt,
>
> I suppose I fixed the patch.
>
> Could you please check which patch is better yours or the attached one?
>
> According to attached benchmark my one is faster for most usual cases,
> but may be I forget something again.
>
> $a["abcdefghij"]   0.130 0.130
> $a["1234567890"]   0.187 0.104
> $a["2147483648"]   0.168 0.142
> $a["0"]            0.116 0.082
> $a["214748364x"]   0.136 0.141
> $a[0]              0.080 0.080

I gotta be away from the computer for a while in a second, so didn't 
actually test your patch myself, but just wanted to say quick that it looks 
good when I skimmed it.  And those benchmarks look cool. :-)

> Also, do you have other patches which I could look into before RC?

I think just the 2 I sent to the list a few days ago, and hoped to get some 
feedback from others (haven't).  If you didn't see them:

http://marc.info/?l=php-internals&m=123704111325725&w=2
This is about double to long conversion, what should happen, consistency, 
etc. that I've been wondering about since last year.  All info should be in 
that message (and linked ones).

http://marc.info/?l=php-internals&m=123722650225792&w=2
Suggestion to have bitwise and modulus operators operate in "64-bit mode" if 
necessary on 32-bit platforms.  Just a bit ago it came to mind that I think 
I can improve this patch -- the behavior would be exactly the same, just 
better, more optimized code.  (I'll do that as soon as I can, but it 
wouldn't affect you looking into it.  I plan to eliminate the big macro I 
added, FYI.)


I'd imagine they could be discussed, etc. past the RC...

> Thanks. Dmitry.

- Matt

> Matt Wilmas wrote:
>> Hi Dmitry,
>>
>> ----- Original Message -----
>> From: "Dmitry Stogov"
>> Sent: Wednesday, March 18, 2009
>>
>>> BTW may be we should eliminate strtol() at all.
>>> There's no need to scan the string twice.
>>
>> Your change to remove strtol() [1] is not checking for overflow
>> correctly (for example, zend_u_strtol()'s checks are more complicated).
>> This breaks handling of keys above ULONG_MAX (it's correct again after
>> ULONG_MAX+LONG_MAX, until ULONG_MAX * 2, etc.).  See:
>>
>> var_dump(array('5000000000' => 1));
>>
>> array(1) {
>>  [705032704]=>
>>  int(1)
>> }
>>
>> And of course the new code is a bit slower for keys that aren't fully
>> numeric, e.g. "123a"
>>
>> [1] http://news.php.net/php.zend-engine.cvs/7465
>>
>>> Dmitry.
>>
>> - Matt
>>