Re: [APC-DEV] IS_INTERNED for apc + zend mixtures

[email protected] (Rasmus Lerdorf) Thu, 28 Jul 2011 08:14:38 -0700
Newsgroups php.apc.dev
Message-ID <[email protected]>
Thanks Dmitry.

I noticed that this introduced 2 new Valgrind warnings:

==23296== Conditional jump or move depends on uninitialised value(s)
==23296==    at 0xBD88BC3: apc_new_interned_string (apc_string.c:82)
==23296==    by 0xBD88FE0: apc_interned_strings_init (apc_string.c:179)
==23296==    by 0xBD824A9: apc_module_init (apc_main.c:845)
==23296==    by 0xBD7A07E: zm_startup_apc (php_apc.c:347)
==23296==    by 0x785C5E: zend_startup_module_ex (zend_API.c:1647)
==23296==    by 0x791E43: zend_hash_apply (zend_hash.c:716)
==23296==    by 0x789909: zend_startup_modules (zend_API.c:1774)
==23296==    by 0x727F1F: php_module_startup (main.c:2103)
==23296==    by 0x82BBAC: php_cli_startup (php_cli.c:414)
==23296==    by 0x82D600: main (php_cli.c:1336)
==23296==
==23296== Conditional jump or move depends on uninitialised value(s)
==23296==    at 0xBD88C8D: apc_new_interned_string (apc_string.c:109)
==23296==    by 0xBD88FE0: apc_interned_strings_init (apc_string.c:179)
==23296==    by 0xBD824A9: apc_module_init (apc_main.c:845)
==23296==    by 0xBD7A07E: zm_startup_apc (php_apc.c:347)
==23296==    by 0x785C5E: zend_startup_module_ex (zend_API.c:1647)
==23296==    by 0x791E43: zend_hash_apply (zend_hash.c:716)
==23296==    by 0x789909: zend_startup_modules (zend_API.c:1774)
==23296==    by 0x727F1F: php_module_startup (main.c:2103)
==23296==    by 0x82BBAC: php_cli_startup (php_cli.c:414)
==23296==    by 0x82D600: main (php_cli.c:1336)

-Rasmus

On 07/28/2011 05:15 AM, Dmitry Stogov wrote:
> Hi Gopal,
> 
> Sorry, I've reverted your patches for PHP and APC and committed the
> proper APC fix.
> 
> The PHP part of your patch made a significant slowdown. (indirect
> function call is much more expensive than inlined comparison and it's
> used quite often e.g. on each HashTable update).
> 
> The APC part of the patch was logically wrong, because it allowed two
> interned strings for the same data.
> 
> Fortunately, the real fix wasn't too difficult. It looks like at some
> point I decided to save a bit more memory by interning class names, but
> that feature wasn't reflected in APC code.
> 
> Thanks. Dmitry.
> 
> 
> On 07/28/2011 02:42 AM, Gopal V wrote:
>> On Wednesday 27 July 2011 01:22 PM, Rasmus Lerdorf wrote:
>>> On 07/27/2011 01:12 PM, Gopal V wrote:
>>>> We probably need an override for that so that we can nest multiple
>>>> interned blocks which are non contiguous between different modules.
>>>
>>> Gopal, can you come up with a suggested override patch for the engine
>>> and we'll get it committed before the alpha-3 release next week.
>>
>> Updated patch for the cases which check IS_INTERNED before interning
>> is initialized.
>>
>> http://notmysock.org/code/zend-is-interned2.patch
>>
>> +
>>
>> http://notmysock.org/code/apc-string-intern.patch
>>
>> Cheers,
>> Gopal
>