[TYPO3-core] RFC #12128: Performance improvement in t3lib_div::get_dirs()

[Ernesto Baschny [cron IT]]

Hi Dan,

Dan Osipov schrieb:
> Ok, I see what you're saying.
> 
> How about this (haven't tested, just thinking):
> 
> try {
>     $d = dir($path);
>     if (is_object($d)) {
>         ...
>     }
> } catch (t3lib_error_ProductionExceptionHandler $e) {
>     // return message
> }
> 
> Right now it returns string 'error' if it fails - so the result will be
> the same... I'm not sure what the effect on performance will be.

I haven't tested, but I expect this won't work as desired, because the
PHP error handler (which turns PHP errors into an exception) is only
activated in "development mode" and not in "production mode", as that
would disrupt many existing sites after the upgrade. And even in
devel-mode it won't turn a warning into an exception.

So you cannot really "catch" anything in this block of code, you can
only either check if $path exists before calling dir() or supress warnings.

In my eyes we could keep it as it is and do it "right" with a new method
which in it's API already states that it can throw an exceptions.

Cheers,
Ernesto

> Dan Osipov
> Calkins Media
> http://danosipov.com/blog/
> 
> Ernesto Baschny [cron IT] wrote:
>> Hi,
>>
>> Dan Osipov schrieb:
>>
>>> <snip>
>>> $d = dir($path);
>>> if (is_object($d)) {
>>> </snip>
>>>
>>> $d is checked to be an object, and if the first line failed, the rest
>>> will not execute.
>>> This was previously mentioned on this list - the @ warning suppression
>>> was required with old PHP versions, but newer versions don't throw such
>>> warnings anymore.
>>>
>>> If you can reproduce a warning, then I'll be happy to insert the @
>>> symbol back.
>>
>> <snip>
>> % php
>> <?php
>> $d = dir('/bogus');
>> ?>
>>
>> Warning: dir(/bogus): failed to open dir: No such file or directory in
>> /home/ernst/- on line 3
>>
>> Call Stack:
>>    10.0437      38896   1. {main}() /home/ernst/-:0
>>    10.0438      38920   2. dir() /home/ernst/-:3
>>
>> </snip>
>>
>> So dir() will output a warning if $path is not existent. You cannot
>> reproduce that?? Which PHP version are you using? I tested on pretty
>> "regular" debian PHP 5.2.0.
>>
>> If course is_object will break following code to execute, but still you
>> will have the warning first.
>>
>>> Another option to make it backwards compatible is use a try/catch block,
>>> and utilize the new Exception handler.
>>
>> Introducing an exception (instead of silently ignoring the non-existing
>> $path) is also not really backwards compatible because it also changes
>> the API. The current API already says it returns the string "error" in
>> case something wents wrong, which is different than throwing an
>> exception.
>>
>> So if you want to make a cool function that throws exceptions and
>> handles this functionality differently, it has to be a new method, and
>> not changing the behaviour of the current method, which has been there
>> for ages. This is a task for the "t3lib_div" split up into a "smaller
>> set" of div-methods, that one would fall into the "filesystem" category.
>>
>> Cheers,
>> Ernesto
>>
>>
>>> Dan Osipov
>>> Calkins Media
>>> http://danosipov.com/blog/
>>>
>>> Ernesto Baschny [cron IT] wrote:
>>>> Hi,
>>>>
>>>> why was the warning output being ignored before? The current API simply
>>>> outputs nothing on "incorrect" $path parameter. With you change,
>>>> warnings might show up where they haven't been before.
>>>>
>>>> So I guess this is not really a solution we can add while we have to
>>>> maintain some backwards compatibility. There has to at least be a check
>>>> if the $path being scanned is really present and a valid directory
>>>> before starting. This refers to the first @dir.
>>>>
>>>> The @ before is_dir is of most probably not required, because is_dir
>>>> should never spit out a warning, it just returns TRUE or FALSE. So this
>>>> can be safely be removed. Although there have been reports on it giving
>>>> out warnings when provided with a huge .iso or .zip file as an
>>>> argument,
>>>> but these are quite old and I couldn't reproduce.
>>>>
>>>> Cheers,
>>>> Ernesto
>>>>
>>>> Dan Osipov schrieb:
>>>>> This is an SVN patch request.
>>>>>
>>>>> Type: Feature
>>>>>
>>>>> Bugtracker references:
>>>>> http://bugs.typo3.org/view.php?id=12128
>>>>>
>>>>> Branch: Trunk
>>>>>
>>>>> Problem:
>>>>> @ symbols are used in the function. They can be removed to improve
>>>>> performance. Performance improvement with the patch is about 5%.
>>>>>
>>>>> I also took the liberty to apply CGL to the function - but if someone
>>>>> can do a better job, feel free.
>>>>>