[wp-trac] [WordPress Trac] #64538: memoize wp_normalize_path

[WordPress Trac]

#64538: memoize wp_normalize_path
--------------------------------------+--------------------------
 Reporter:  josephscott               |       Owner:  (none)
     Type:  defect (bug)              |      Status:  new
 Priority:  normal                    |   Milestone:  7.0
Component:  General                   |     Version:
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch has-unit-tests  |     Focuses:  performance
--------------------------------------+--------------------------

Comment (by dmsnell):

 @josephscott this is wonderful — thank you for sharing your benchmark.
 with it, I was able to investigate a few more things.

  - as noted in the comment above, the `while ()` loop replacing `//`
 seemed to be the heaviest bottleneck.
  - it seems like all the string processing is indeed the main source of
 delay. skipping that, via the cache, is exactly what’s lifting the overall
 performance here.
  - with a warm JIT, the cacheless optimization with my two changes above
 brought it within a little over 2x the runtime of the cached version. the
 overall performance of the non-cache version was between 25% and 45%
 faster than the original.

 I even wondered about the odd `generate_paths()` functions and if we could
 have been skewing the results due to pre-computing the string hashes in
 `generate_paths()` outside of the timing function. That is, array lookup
 is much cheaper if that hash has already been computed. The cached version
 was still faster, though the gap was reduced when I moved this into the
 benchmark. In order to facilitate this I also precomputed a large list of
 paths as a newline-delimited string and had `generate_paths()` randomly
 point into that string to grab a line.

 you have exhausted all of the cases I can think of, and I wonder how
 generally this approach could apply to other Core functions which see the
 same strings frequently. there are probably many out there that do, where
 a limited cache would serve us better than uncapped or large caches.

 ----

 the one thing I think is worth reflecting on are the cases where a given
 path loads before a stream wrapper is registered. there seemed to mostly
 be a couple stream wrappers in the plugin directory,
 [https://docs.guzzlephp.org/en/5.3/streams.html Guzzle] being one of them.
 I foresee the failure scenario being the case that we normalize a path
 before the plugin code is activated, and then that path stays in memory.

 it does seem low-risk, as one might expect plugin code to run before
 processing the rest of the request, so as long as those `require` or
 `require_once` statements don’t appear dynamically and on-demand, this
 should not misreport the paths. regardless, this is what I believe to be a
 bug anyway, so that point will be moot if we fix the bug, as the actual
 cached value will then //only// rely on the normalized path and not on
 `stream_get_wrappers()`

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