Improve _get_block_templates_paths performace #5281
Conversation
Internally RecursiveDirectoryIterator has a file check, Ref: https://github.com/php/php-src/blob/62e2402534d5e51faba41e30c79f8e4af1783370/ext/spl/spl_directory.c#L1475 `spl_filesystem_is_invalid_or_dot`
There was a problem hiding this comment.
Nice idea! I'm not seeing a big performance difference in the automated Performance Test run, but that's likely due to this issue not really effecting the TT3 theme that the block-theme tests are based on.
I see that all of the unit tests are currently failing, and that's likely due to the removal of the file_exists check. I wonder if wrapping that code in a try/catch block will fix those tests.
Let's get some benchmarking on this change, but seems sensible. We may even want to consider caching this persistently in the future.
kt-12
changed the title
|
TT4 Homepage performance metrics. Conducted 500 rounds 3 times and got the mean. This is a bit contradictory to what was observed in Blackfire.
|
|
360effb Fixes the test failures and I've run some benchmarks locally which do show some small potential improvement, but not huge. Likely due to most of the performance impact being incurred the first time the function is called. However, there is a decent memory improvement by avoiding redundant calls to the Iterator class methods. Not opposed to committing this enhancement, but I do think focusing on the parent function, Trunk w/ PR applied |
There was a problem hiding this comment.
@kt-12 While the performance gain here is very small, I think the enhancement is simple enough to go for it, plus there's the memory usage benefit.
Since there are no tests for _get_block_template_paths(), could you add some basic coverage? Probably simply two tests are enough, one for a directory that exists, another one for a directory that doesn't exist.
There was a problem hiding this comment.
Thanks @kt-12 for the PR. Left some feedback for your consideration.
Remove extra line Co-authored-by: Mukesh Panchal <[email protected]>
Trac ticket: https://core.trac.wordpress.org/ticket/58196
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.