Hi, First thanks a lot for your quick review ;)
I subscribed to the pecl-dev list, but have not yet got any response, maybe something is temporarily broken there? So, now about the profiling extension: License would be the php license, or anything that suits ;) The INI-Options have a reason. The profiler should report compile times for the PHP files, and at the point where profiler_start() is called, the files are already compiled. One solution would be to always profile, which I don't want because it is a (small) performance impact, even if you do not use the profiler. Another solution, which I'm going to implement, is to always profile the first call to zend_compile_file, and search the resulting zend_op_array for a call to profiler_quickdump. If it's found, profiling is turned on. The popup is optional, reason for it is that some pages are designed in a way that additional data at the bottom of the page is either not readable or is displayed in strange optics caused by stylesheets. I think a popup is a nice solution for this, if people need it. I changed the name to "profiler", phprofiler was really hard to type ;) I fixed the segfault, and will go through the source again checking it for security. Moritz -----Original Message----- From: Antony Dovgal [mailto:[EMAIL PROTECTED] Sent: Saturday, May 20, 2006 11:56 PM To: Moritz Möller Cc: internals@lists.php.net; [EMAIL PROTECTED] Subject: Re: [PHP-DEV] profiling extension and minimal-php-sapi You need to contact [EMAIL PROTECTED] to apply for PECL account. I CC'ed this list. On 21.05.2006 01:16, Moritz Möller wrote: > Hello, > I made two extensions for PHP that Im want to share, but I do not really > know how to do it ;) Basically you've already done (almost) everything. You also need to clarify which license you're going to use for those extensions (from what I can see it's PHP License and it's fine). > One is a profiling extension, called PhProfile: > > - works with zend optimizer and similar (as opposed to xdebug) > > - counts the number of function calls and the total send self time > for each function > > - usable either by calling phprofile_start(); at the beginning and > print_r(phprofile_get()) at the end of the script or > > - by setting up a dump-profile parameter in your php.ini (like > phprofile.quick_dump_key=prof) and appending the parameter prof to the url > (script.php?prof=1). If done, a html table with all function calls sorted by > self time is appended to the page. > > - If phprofile.quick_dump_mode is set to 2, the profiling data is > displayed in a popup Personally, I like the idea of this one. But there are several things I'd like to change: 1) I don't see any sense in these INI options. I think it's pretty easy to add if (isset($_REQUEST['profiler_or_anything you like'])) profiler_start(); And having an extension that uses popups (which every browser blocks nowadays) is not really good idea too IMO. Both Javascript and HTML do not look really good in console.. 2) I don't like the name =). IMO "profiler" is enough. This applies to functions too, of course. I didn't look into the implementation too close, but there are several problems I've spotted: 1) Did you try to run phprofiler_get() before phprofiler_start()? You should get a segfault, because PHPROFILER_G(data) is not initialized at this point. Btw, there is profiler_start(), but no profiler_end/stop()? 2) You need to make the code to comply with PECL coding standards if you want it to be accepted. Windows EOLs, one-line IF's are no good.. 3) This code won't compile with MSVC and gcc 2.9x: static void phprofiler_zend_execute_internal(zend_execute_data* execute_data_ptr, int return_value_used TSRMLS_DC) { if (!PHPROFILER_G(enabled)) { phprofiler_saved_zend_execute_internal(execute_data_ptr, return_value_used TSRMLS_CC); return; } const char* function_name = execute_data_ptr->function_state.function->common.function_name; ... } You have to move "const char*" declaration to the beginning to of the func. -- Wbr, Antony Dovgal -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
