Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:126165
Feedback-ID: id5114917:Fastmail
Content-Type: multipart/alternative;
 boundary="------------qJzVP1LXXQhA60EJK3676iav"
Message-ID: <22132ea8-9e5b-4688-9bf4-4a1972e347b1@rwec.co.uk>
Date: Sun, 22 Dec 2024 12:16:48 +0000
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PHP-DEV] Discussion: Remove file statcache?
To: internals@lists.php.net
References: <27531d9d-9bfe-4acc-b9ab-80b1017e3038@app.fastmail.com>
 <CAEKnhAFU32EXh483QzR0LLUt+Jou4ejKjOXqBQ_G43wR7LW5cA@mail.gmail.com>
 <ACB1A7C2-FAF5-4A43-BBB9-A3E8469E2385@rwec.co.uk>
 <21dd9ac7-e81b-4509-bfa4-a36d05237270@gmail.com>
 <F31E457C-B04D-41D3-A544-0A1E7F139261@rwec.co.uk>
 <614d3f45-0f54-4a2e-a21e-2eec5f496fa0@gmail.com>
Content-Language: en-GB
In-Reply-To: <614d3f45-0f54-4a2e-a21e-2eec5f496fa0@gmail.com>
From: imsop.php@rwec.co.uk ("Rowan Tommins [IMSoP]")

This is a multi-part message in MIME format.
--------------qJzVP1LXXQhA60EJK3676iav
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 21/12/2024 20:50, Niels Dossche wrote:
> Adding a parameter for a cache, which should've been transparent in the first place, to every file operation is messy.


I would say it's less messy than having to work out when to turn a 
global setting on or off. In particular, it would be horrible for shared 
libraries, the equivalent of the above would be something like this:

$old_cache_setting = ini_set('enable_stat_cache', 1);
$perms = fileperms($name); $size = filesize($name); 
ini_set('enable_stat_cache', $old_cache_setting);

Similarly, for the false case, library code would either have to assume 
the cache might be enabled, and call clearstatcache() just in case; or 
it would have to carefully wrap code in similar ini_set blocks.

As far as I can see, both code that benefits from the cache, and code 
that suffers from it, is very rare; but if you know you're writing one 
or the other, having an explicit way to mark *that code* seems more 
appropriate than toggling a global setting.


> But if you want to add extra parameters to functions that can potentially touch the stat cache, then you need to take into account spl as well. Adding extra parameters to the functions in those classes are a BC break because the signature of potential userland function overrides would no longer be compatible at compile time.


Ah yes, I hadn't thought of objects being affected. On the other hand, 
objects have an obvious place to store both the state of the setting and 
the cache itself: on the instance.

For example, a local rather than global cache would allow this to make 
two stat calls, rather than four:

$file1 = new SplFileInfo($name1, usecache: true);
$file2= new SplFileInfo($name2, usecache: true);
if (
     $file1->getSize() !== $file2->getSize()
     || $file1->getMTime() !== $file2->getMTime()
) { ... }


In fact, it would probably be useful to pre-fetch a snapshot in the 
constructor, rather than just caching on the first method call, so that 
this worked:

$before = new SplFileInfo($name, snapshot: true);
do_something();
$after = new SplFileInfo($name, snapshot: true);
if ( $before->getSize() !== $after->getSize() ) { ... }


Inheritance of constructors isn't restricted, so that would not be a BC 
break, and seems both more powerful and easier to understand than the 
current feature.


Regards,

-- 
Rowan Tommins
[IMSoP]

--------------qJzVP1LXXQhA60EJK3676iav
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit

<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <div class="moz-cite-prefix">On 21/12/2024 20:50, Niels Dossche
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:614d3f45-0f54-4a2e-a21e-2eec5f496fa0@gmail.com">
      <pre wrap="" class="moz-quote-pre">Adding a parameter for a cache, which should've been transparent in the first place, to every file operation is messy.</pre>
    </blockquote>
    <p><br>
    </p>
    <p>I would say it's less messy than having to work out when to turn
      a global setting on or off. In particular, it would be horrible
      for shared libraries, the equivalent of the above would be
      something like this:</p>
    <p><span class="blob-code-inner blob-code-marker "
        data-code-marker="+"><span class="pl-s">$old_cache_setting =
          ini_set('enable_stat_cache', 1);<br>
        </span></span><span style="white-space: pre-wrap">$perms = fileperms($name);
</span><span style="white-space: pre-wrap">$size = filesize($name);
ini_set('enable_stat_cache', $old_cache_setting);
</span></p>
    <p>Similarly, for the false case, library code would either have to
      assume the cache might be enabled, and call clearstatcache() just
      in case; or it would have to carefully wrap code in similar
      ini_set blocks.</p>
    <p>As far as I can see, both code that benefits from the cache, and
      code that suffers from it, is very rare; but if you know you're
      writing one or the other, having an explicit way to mark *that
      code* seems more appropriate than toggling a global setting.<br>
    </p>
    <p><br>
    </p>
    <blockquote type="cite"
      cite="mid:614d3f45-0f54-4a2e-a21e-2eec5f496fa0@gmail.com">
      <pre wrap="" class="moz-quote-pre">
But if you want to add extra parameters to functions that can potentially touch the stat cache, then you need to take into account spl as well. Adding extra parameters to the functions in those classes are a BC break because the signature of potential userland function overrides would no longer be compatible at compile time.</pre>
    </blockquote>
    <p><br>
    </p>
    <p>Ah yes, I hadn't thought of objects being affected. On the other
      hand, objects have an obvious place to store both the state of the
      setting and the cache itself: on the instance.</p>
    <p>For example, a local rather than global cache would allow this to
      make two stat calls, rather than four:</p>
    <p>$file1 = new SplFileInfo($name1, usecache: true);<br>
      $file2= new SplFileInfo($name2, usecache: true);<br>
      if (<br>
          $file1-&gt;getSize() !== $file2-&gt;getSize()<br>
          || $file1-&gt;getMTime() !== $file2-&gt;getMTime()<br>
      ) { ... }</p>
    <p><br>
    </p>
    <p>In fact, it would probably be useful to pre-fetch a snapshot in
      the constructor, rather than just caching on the first method
      call, so that this worked:<br>
    </p>
    <p>$before = new SplFileInfo($name, snapshot: true);<br>
      do_something();<br>
      $after = new SplFileInfo($name, snapshot: true);<br>
      if ( $before-&gt;getSize() !== $after-&gt;getSize() ) { ... }<br>
    </p>
    <p><br>
    </p>
    <p>Inheritance of constructors isn't restricted, so that would not
      be a BC break, and seems both more powerful and easier to
      understand than the current feature.<br>
    </p>
    <p><br>
    </p>
    <p>Regards,<br>
    </p>
    <pre class="moz-signature" cols="72">-- 
Rowan Tommins
[IMSoP]</pre>
  </body>
</html>

--------------qJzVP1LXXQhA60EJK3676iav--