On Tue, Jan 7, 2025, at 1:21 PM, Niels Dossche wrote: > On 07/01/2025 19:49, Ilija Tovilo wrote: >> I wouldn't necessarily call this a workaround, but more of a missed >> optimization (one branch for each static property write that is >> unlikely to mispredict). If you wish, I can have a look at separating >> cache slots. This may lead to a slowdown due to cache priming (as only >> R/IS and RW/W/UNSET will be shared). To be honest, I doubt either of >> those will lead to a measurable difference in real code. I can confirm >> this by running some benchmarks. The benefit of separate cache slots >> is that it can be entirely handled in >> zend_fetch_static_property_address(), which would reduce VM/ future >> JIT changes, although the separation of the cache slots themselves >> might be more complex (given how it's currently implemented). >> > > I think splitting cache slots will indeed be worse than what it is now. > However, that argument in itself just doesn't really convince me > honestly. > You are likely right about the branch prediction, but the CPU still has > to do some extra work; and it's also just more things to think about as > a maintainer. > I'll think about it more, maybe I'll just abstain from voting. > > Kind regards > Niels
Late follow-up here. Ilija finally managed to squeeze in time to benchmark this. Synthetic benchmarks were inconclusive. A practical benchmark using Symfony Demo showed the code from this patch as 0.03% faster than HEAD, which is within the margin of error of the benchmark, so it's basically a wash/no-impact. With that settled, I will be opening the vote on this RFC tomorrow sometime, baring any last second questions or complications. --Larry Garfield
