[wp-trac] [WordPress Trac] #55069: Optimize POMO_FileReader.read_all() using stream_get_contents()

[WordPress Trac]

#55069: Optimize POMO_FileReader.read_all() using stream_get_contents()
---------------------------+-----------------------------
 Reporter:  maxkellermann  |       Owner:  SergeyBiryukov
     Type:  enhancement    |      Status:  accepted
 Priority:  normal         |   Milestone:  6.0
Component:  I18N           |     Version:
 Severity:  normal         |  Resolution:
 Keywords:  has-patch      |     Focuses:  performance
---------------------------+-----------------------------

Comment (by maxkellermann):

 > would that lead to PHP warnings on some installs which had none
 previously

 You are right. It seems PHP people very much like the idea of sprinkling
 unnecessary `access()` and `stat()` system calls everywhere.

 I'm not a PHP programmer; I do lower-level code, and one very simple idea
 to make code faster is to omit stuff that's not necessary. For example,
 what's the point of doing `access()` to check if a file exists before
 opening it; just `open()` it and then you can still handle the same
 `ENOENT`. Doing system calls has become more expensive recently due to the
 Meltdown/Spectre mitigations, and avoiding them does make a difference.

 Additionally, the extra `access()`/`stat()` is buggy; it's a TOCTOU bug.
 It's not only slower, but also unreliable.

 Knowing that, it is surprising that PHP punishes fast and reliable code
 with a warning, which is a very visible signal to their users: this
 software is bad. That's sad.

 I can understand if you don't want to merge my remaining patch for that
 reason. But if you do want it, I'll investigate what else can be optimized
 that way and send more patches.

 Maybe this kind of warning can be suppressed? I don't know, I'm not a PHP
 expert.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/55069#comment:13>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform