Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:81557 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 73110 invoked from network); 2 Feb 2015 07:49:03 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 2 Feb 2015 07:49:03 -0000 Authentication-Results: pb1.pair.com smtp.mail=anatol.php@belski.net; spf=permerror; sender-id=unknown Authentication-Results: pb1.pair.com header.from=anatol.php@belski.net; sender-id=unknown Received-SPF: error (pb1.pair.com: domain belski.net from 85.214.73.107 cause and error) X-PHP-List-Original-Sender: anatol.php@belski.net X-Host-Fingerprint: 85.214.73.107 klapt.com Received: from [85.214.73.107] ([85.214.73.107:45373] helo=h1123647.serverkompetenz.net) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id C2/11-02376-EEB2FC45 for ; Mon, 02 Feb 2015 02:49:03 -0500 Received: by h1123647.serverkompetenz.net (Postfix, from userid 33) id 5852D23D6002; Mon, 2 Feb 2015 08:48:59 +0100 (CET) Received: from 87.159.48.115 (SquirrelMail authenticated user anatol@belski.net) by webmail.klapt.com with HTTP; Mon, 2 Feb 2015 08:48:59 +0100 Message-ID: <3d55a2ddaccd53493d3b32626cff1ff8.squirrel@webmail.klapt.com> In-Reply-To: References: <004e01d03eb4$a4dd8240$ee9886c0$@tekwire.net> Date: Mon, 2 Feb 2015 08:48:59 +0100 To: "Xinchen Hui" Cc: "Anatol Belski" , "francois" , "PHP Internals" , "Dmitry Stogov" User-Agent: SquirrelMail/1.5.2 [SVN] MIME-Version: 1.0 Content-Type: text/plain;charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: Re: [PHP-DEV] [DICUSS]Cleanup resource handling APIs From: anatol.php@belski.net ("Anatol Belski") Hi Hui, On Mon, February 2, 2015 08:40, Xinchen Hui wrote: > Hey: > > > On Mon, Feb 2, 2015 at 3:35 PM, Anatol Belski > 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 >>> >>> 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) == >>>>> 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. > > yeah, I see, u suggest even more so that int thing to be removed from the struct. But probably one could go this way then - first cleanup the signature, stabilize. That would give us the ground to localize the ->handle use cases and remove it in the next step. Kind of step by step doing. Regards Anatol