Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:43396
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 212.25.124.163 as permitted sender)
Message-ID: <49C141D1.9020308@zend.com>
Date: Wed, 18 Mar 2009 21:47:45 +0300
User-Agent: Thunderbird 2.0.0.19 (X11/20090105)
MIME-Version: 1.0
To: Matt Wilmas <php_lists@realplain.com>
CC: internals@lists.php.net, 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> <31133F79731047CB9918A6D54FB908EC@pc1>
In-Reply-To: <31133F79731047CB9918A6D54FB908EC@pc1>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH] Bug #45877: LONG_MAX/MIN array key as string/int
From: dmitry@zend.com (Dmitry Stogov)



Matt Wilmas wrote:
> 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. :-)

ok. I'll wait till tomorrow.

>> 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'll try to look into them too.

Thanks. Dmitry.

> 
> 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
>>>
>