Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:126370
Feedback-ID: i8414410d:Fastmail
Precedence: bulk
MIME-Version: 1.0
Date: Mon, 10 Feb 2025 21:58:41 -0600
To: "php internals" <internals@lists.php.net>
Message-ID: <ee23499f-6edf-4c67-8c6a-f9f7412afd1a@app.fastmail.com>
In-Reply-To: <8c53b0fe-cafe-406c-8bab-1cedf2271c2f@gmail.com>
References: <0addc1b4-1e4d-45a9-a289-5f9a8e1da692@app.fastmail.com>
 <fdea1691-d1b6-44bb-88a7-71d0c0deb7bb@app.fastmail.com>
 <ba402e4d-51b4-4a4c-ae57-380563eb2e3e@gmail.com>
 <CAPyj-LBv=AV-pcLv1vSsohY-HM4zrWmB6j4ZP5TnVhj_VfYznw@mail.gmail.com>
 <8c53b0fe-cafe-406c-8bab-1cedf2271c2f@gmail.com>
Subject: Re: [PHP-DEV] [RFC] Static property asymmetric visibility
Content-Type: text/plain
Content-Transfer-Encoding: 7bit
From: larry@garfieldtech.com ("Larry Garfield")

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