Hey: On Mon, Feb 2, 2015 at 3:35 PM, Anatol Belski <anatol....@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çois Laupretre <franc...@tekwire.net> >> wrote: >> >>>> De : Xinchen Hui [mailto:larue...@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) == >>>> 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; const >> char *class_name; >> >> if (default_id==-1) { /* use id */ if (!passed_id) { if >> (resource_type_name) { >> class_name = 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) != IS_RESOURCE) { // < === what >> are this? if (resource_type_name) { class_name = >> 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, const >>>> 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 = 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=php-internals&m=142006455308305&w=2 , 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 suggestion > 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 it > 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 > -- Xinchen Hui @Laruence http://www.laruence.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
