Re: [RFC] Static property asymmetric visibility
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
Thread (11 messages)