Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:81556
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 209.85.215.43 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <d51d824fc37b85b3a29d1e175760fb39.squirrel@webmail.klapt.com>
References: <CABBJUpdzJZFYwBh0trYMTFSBCDrjH66nJFrBRNhCQTGrpTRDgQ@mail.gmail.com>
 <004e01d03eb4$a4dd8240$ee9886c0$@tekwire.net> <CABBJUpcWLYVp=D-0Zc79YhWhAhQCMGNKFN1Av5fU2rHY-7xYAA@mail.gmail.com>
 <d51d824fc37b85b3a29d1e175760fb39.squirrel@webmail.klapt.com>
Date: Mon, 2 Feb 2015 15:40:28 +0800
Message-ID: <CABBJUpen5eGYn_Mf35iu4r84bRC4WhRVxn2umk1U_7YRy_FRtw@mail.gmail.com>
To: Anatol Belski <anatol.php@belski.net>
Cc: francois <francois@tekwire.net>, PHP Internals <internals@lists.php.net>, 
	Dmitry Stogov <dmitry@zend.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] [DICUSS]Cleanup resource handling APIs
From: laruence@php.net (Xinchen Hui)

Hey:

On Mon, Feb 2, 2015 at 3:35 PM, Anatol Belski <anatol.php@belski.net> wrote=
:
> Hi,
>
> On Mon, February 2, 2015 08:11, Xinchen Hui wrote:
>> Hey:
>>
>>
>> On Mon, Feb 2, 2015 at 2:51 PM, Fran=C3=A7ois Laupretre <francois@tekwir=
e.net>
>> wrote:
>>
>>>> De : Xinchen Hui [mailto:laruence@php.net]
>>>> we used to use lval of zval as a handle to access resource type..
>>>>
>>>> but now, we introduced a new type IS_RESOURCE, which make the
>>>> handle(id) sort of redundant .
>>>
>>> Wrong. The IS_RESOURCE type has nothing to do with PHP 7. Only
>>> zend_resource is new. And handle is not redundant.
>> of course it's a typo. I meant zend_resource
>>>
>>>> further more, the common usage when handling resource is like:
>>>>
>>>> if (zend_parse_parameters(ZEND_NUM_ARGS(), "rl", &result, &offset) =3D=
=3D
>>>> FAILURE) {
>>>> return; }
>>>> ZEND_FETCH_RESOURCE(mysql_result, MYSQL_RES *, result, -1, "MySQL
>>>> result", le_result);
>>>>
>>>> as you can see, we use "r" to receive a IS_RESOURCE type, that means,
>>>> check the type in ZEND_FETCH_RESOURCE is overhead..
>>>
>>> There's no overhead here. Zend_parse_parameters checks that received
>>> arg is IS_RESOURCE. Fetch then checks that received resource is one of
>>> the accepted resource types. Sorry to say that, but are you sure you
>>> understand the difference between zval types and resource types ?
>> ..... do you really read the FETCH_RESOURCE?
>>
>>
>> ZEND_API void *zend_fetch_resource(zval *passed_id, int default_id,
>> const char *resource_type_name, int *found_resource_type, int
>> num_resource_types, ...) {
>> int actual_resource_type; //  void *resource;
>> va_list resource_types; int i; zend_resource *res; const char *space; co=
nst
>> char *class_name;
>>
>> if (default_id=3D=3D-1) { /* use id */ if (!passed_id) { if
>> (resource_type_name) {
>> class_name =3D get_active_class_name(&space); zend_error(E_WARNING,
>> "%s%s%s(): no %s resource
>> supplied", class_name, space, get_active_function_name(),
>> resource_type_name); }
>> return NULL; } else if (Z_TYPE_P(passed_id) !=3D IS_RESOURCE) { // < =3D=
=3D=3D what
>> are this? if (resource_type_name) { class_name =3D
>> get_active_class_name(&space); zend_error(E_WARNING, "%s%s%s(): supplied
>> argument is not a valid %s resource", class_name, space,
>> get_active_function_name(), resource_type_name); }
>> return NULL; }
>>
>>
>>
>>>
>>>> ZEND_API void *zend_fetch_resource(zval *passed_id, int default_id,
>>>> const char *resource_type_name, int *found_resource_type, int
>>>> num_resource_types, ...)
>>>>
>>>> we use va_args to passing resource type, that means, the rescue type
>>>> arguments can not be passed by register but by stack.. which is a
>>>> little low effiicient .
>>>
>>> What do you mean with 'rescue' type ?
>>>
>> expected resource_type
>>>
>>> Fetch is supposed to check for a variable number of possible resource
>>> types. It could probably be restricted to 2 possible types as,
>>> generally, it is the maximum (one for non-persistent, one for
>>> persistent). But I am not sure the overhead of passing arg on the stack
>>> justifies a change. Remember that id is searched in an array, which
>>> takes probably much more time that pushing/popping one or two
>>> arguments.
>>>
>>>> so, I'd like propose a zend_resource handling API cleanup..
>>>>
>>>> 1.  DROP ZEND_REGISTER_RESOURCE/FETCH_RESOURCE.
>>>>
>>>>
>>>> 2.  add :
>>>>
>>>>
>>>> ZEND_API void *zend_fetch_resource(zend_resource *res, const
>>>> char *resource_type_name, int resource_type); ZEND_API void
>>>> *zend_fetch_resource2(zend_resource *res, const char
>>>> *resource_type_name, int *found_type, int resource_type, int
>>>> resource_type2); ZEND_API void *zend_fetch_resource_ex(zval *res, cons=
t
>>>> char *resource_type_name, int resource_type);
>>>> ZEND_API void *zend_fetch_resource2_ex(zval *res, const char
>>>> *resource_type_name, int *found_type, int resource_type, int
>>>> resource_type2);
>>>
>>> If you drop ZEND_REGISTER_RESOURCE, how do you register new resources ?
>>> Or do you mean you don't register them any more ? But registering them
>>> is mandatory if we want them to be freed when request ends.
>>>
>>>> furthermore, I'd like to discuss remove the handle in zend_resource
>>>> struct..
>>>>
>>>> it may breaks some usage (use resource as long/double/string)
>>>>
>>>> case IS_RESOURCE: { char buf[sizeof("Resource id #") +
>>>> MAX_LENGTH_OF_LONG];
>>>> int len;
>>>>
>>>> len =3D snprintf(buf, sizeof(buf), "Resource id #" ZEND_LONG_FMT,
>>>> (zend_long)Z_RES_HANDLE_P(op));
>>>> return zend_string_init(buf, len, 0); }
>>>>
>>>
>>> OK. You want to remove resource registration. But resources don't work
>>> this way (see
>>> http://devzone.zend.com/446/extension-writing-part-iii-resources/). If
>>> you remove the handle, you remove the whole zend_list API.
>>>
>>> The zend_resource struct is not a structure you may fill with random
>>> data. Using the handle to store long/double/string is not a legitimate
>>> usage.
>> ....I think you don't understand what I am talking about. sorry
>>
>>
> I was writing about it last year as well
> http://marc.info/?l=3Dphp-internals&m=3D142006455308305&w=3D2 , slightly
> different idea. Annd there is also a short piece of code to illustrate:
> https://gist.github.com/weltling/9367db5e242ef7e60042 ...
>
> The integer resource looks like is needed at some places, so my suggestio=
n
> was to split zend_fetch_resource function like illustrated in the patch.
> At 99% of places that integer link is just passed as -1, so then having i=
t
> be present always in the signature is just a waste of "resources" :)
for now, we still keep the handle which could be accessed by Z_RES_HANDLE_P

I just talked about remove it later maybe.

thanks
>
> Regards
>
> Anatol
>



--=20
Xinchen Hui
@Laruence
http://www.laruence.com/