Zend_Loader::isReadable fix

6 messages Options
Embed this post
Permalink
chielsen

Zend_Loader::isReadable fix

Reply Threaded More More options
Print post
Permalink
I'm getting annoyed by the errors the isReadable function gives because it's trying to open non-existing files (which get suppressed).
Why not use file_exists in conjuction with file_open? It won't give (as many) errors with the same implementation.

There was an alternative fix, but it got revert because it broke the BC (which is what?).

SO:

    public static function isReadable($filename)
    {
        if (!file_exists($filename) || !$fh = @fopen($filename, 'r', true)) {
            return false;
        }
        @fclose($fh);
        return true;
    }
Thomas Weidner

Re: Zend_Loader::isReadable fix

Reply Threaded More More options
Print post
Permalink
BC is Backwards Compatibiliy

Greetings
Thomas Weidner, I18N Team Leader, Zend Framework
http://www.thomasweidner.com

----- Original Message -----
From: "chielsen" <[hidden email]>
To: <[hidden email]>
Sent: Tuesday, November 04, 2008 5:57 PM
Subject: [fw-core] Zend_Loader::isReadable fix


>
> I'm getting annoyed by the errors the isReadable function gives because
> it's
> trying to open non-existing files (which get suppressed).
> Why not use file_exists in conjuction with file_open? It won't give (as
> many) errors with the same implementation.
>
> There was an alternative fix, but it got revert because it broke the BC
> (which is what?).
>
> SO:
>
>    public static function isReadable($filename)
>    {
>        if (!file_exists($filename) || !$fh = @fopen($filename, 'r', true))
> {
>            return false;
>        }
>        @fclose($fh);
>        return true;
>    }
> --
> View this message in context:
> http://www.nabble.com/Zend_Loader%3A%3AisReadable-fix-tp20326421p20326421.html
> Sent from the Zend Core mailing list archive at Nabble.com.

weierophinney

Re: Zend_Loader::isReadable fix

Reply Threaded More More options
Print post
Permalink
In reply to this post by chielsen
-- chielsen <[hidden email]> wrote
(on Tuesday, 04 November 2008, 08:57 AM -0800):
>
> I'm getting annoyed by the errors the isReadable function gives because it's
> trying to open non-existing files (which get suppressed).
> Why not use file_exists in conjuction with file_open? It won't give (as
> many) errors with the same implementation.
>
> There was an alternative fix, but it got revert because it broke the BC
> (which is what?).

There are two things going on here.

First off, BC == Backwards Compatibility. If a new implementation does
not work the same as the original -- i.e., given the same input, does
not return the same results -- we cannot make the change while in the
same major revision.

Second, using file_exists + fopen() means multiple stat calls to the
filesystem, which degrades performance. We can't use file_exists() by
itself, as it is not include_path aware; fopen() *is*, however.
Considering the number of reports about performance, adding additional
stat calls is a no-go.

There are ways to remove the warnings from your logs. First, you can add
an error handler routine that greps for them and removes them. Second,
in 1.7.0 (and current svn trunk), you can try using the PluginLoader
include file cache. This should reduce the number of isReadable() calls
tremendously, as the PluginLoader is the biggest consumer of it.

> SO:
>
>     public static function isReadable($filename)
>     {
>         if (!file_exists($filename) || !$fh = @fopen($filename, 'r', true))
> {
>             return false;
>         }
>         @fclose($fh);
>         return true;
>     }

--
Matthew Weier O'Phinney
Software Architect       | [hidden email]
Zend Framework           | http://framework.zend.com/
chielsen

Re: Zend_Loader::isReadable fix

Reply Threaded More More options
Print post
Permalink

Matthew Weier O'Phinney-3 wrote:
Second, using file_exists + fopen() means multiple stat calls to the
filesystem, which degrades performance. We can't use file_exists() by
itself, as it is not include_path aware; fopen() *is*, however.
Considering the number of reports about performance, adding additional
stat calls is a no-go.
Thanks for the explanation.
Has a change to the php is_readable() already been recommended?
weierophinney

Re: Zend_Loader::isReadable fix

Reply Threaded More More options
Print post
Permalink
-- chielsen <[hidden email]> wrote
(on Tuesday, 04 November 2008, 11:34 AM -0800):
> Matthew Weier O'Phinney-3 wrote:
> > Second, using file_exists + fopen() means multiple stat calls to the
> > filesystem, which degrades performance. We can't use file_exists() by
> > itself, as it is not include_path aware; fopen() *is*, however.
> > Considering the number of reports about performance, adding additional
> > stat calls is a no-go.
>
> Thanks for the explanation.
> Has a change to the php is_readable() already been recommended?

I believe so, but we wouldn't be able to use it until the version in
which it is introduced is at or below our minimum supported version --
which could be months or years down the road, if indeed it's ever added.

--
Matthew Weier O'Phinney
Software Architect       | [hidden email]
Zend Framework           | http://framework.zend.com/
chielsen

Re: Zend_Loader::isReadable fix

Reply Threaded More More options
Print post
Permalink
I think checking for the right version will take alot less than writing a log.

Matthew Weier O'Phinney-3 wrote:
-- chielsen <michiel@thalent.nl> wrote
(on Tuesday, 04 November 2008, 11:34 AM -0800):
> Matthew Weier O'Phinney-3 wrote:
> > Second, using file_exists + fopen() means multiple stat calls to the
> > filesystem, which degrades performance. We can't use file_exists() by
> > itself, as it is not include_path aware; fopen() *is*, however.
> > Considering the number of reports about performance, adding additional
> > stat calls is a no-go.
>
> Thanks for the explanation.
> Has a change to the php is_readable() already been recommended?

I believe so, but we wouldn't be able to use it until the version in
which it is introduced is at or below our minimum supported version --
which could be months or years down the road, if indeed it's ever added.

--
Matthew Weier O'Phinney
Software Architect       | matthew@zend.com
Zend Framework           | http://framework.zend.com/