ZEND_ACC_* flags

php.internals

Dmitry Stogov

8 years ago
Hi, I tried to fix ZEND_ACC_* flags mess. https://gist.github.com/dstogov/3b6ae377c17524b219670960cf98f8c1 The patch specifies flags meaning, and reorder them according to meaning and frequency of usage (this allows generation of shorter instructions on x86). Unfortunately, the patch breaks few reflection based tests that relay on binary modifiers values. Do you think, it it's OK to commit thin into 7.3 or better to wait for branching? Thanks. Dmitry.

Marco Pivetta

8 years ago
This can potentially break some cached flags somewhere, and is a BC break: any rationale behind the change? On Wed, 25 Jul 2018, 17:01 Dmitry Stogov, <dmitry@zend.com> wrote:

Stas Malyshev

8 years ago
Hi!
> I tried to  fix ZEND_ACC_* flags mess. > > > https://gist.github.com/dstogov/3b6ae377c17524b219670960cf98f8c1 > > > The patch specifies flags meaning, and reorder them according to meaning > and frequency of usage (this allows generation of shorter instructions > on x86). > > Unfortunately, the patch breaks few reflection based tests that relay on > binary modifiers values.
I am not sure I understand what the effect is here - does it allow any measurable improvement? If not, I'd wait until the after the branch. But I could change my mind if there's real performance gain... The test part, however, which replaces bits with constants, I think is good anytime, I don't think we should have been using bit values anyway.
-- Stas Malyshev smalyshev@gmail.com

Dmitry Stogov

8 years ago
OK. I'll keep this for next PHP version. There is no any performance improvement (just a bit smaller code). The patch clarifies the meaning of all the flags, what entities use them, and points what some of them may be eliminated completely. Thanks. Dmitry. ________________________________ From: Stanislav Malyshev <smalyshev@gmail.com> Sent: Wednesday, July 25, 2018 9:35:11 PM To: Dmitry Stogov; Christoph M. Becker; PHP internals list Cc: Nikita Popov Subject: Re: ZEND_ACC_* flags Hi!
> I tried to fix ZEND_ACC_* flags mess. > > > https://gist.github.com/dstogov/3b6ae377c17524b219670960cf98f8c1 > > > The patch specifies flags meaning, and reorder them according to meaning > and frequency of usage (this allows generation of shorter instructions > on x86). > > Unfortunately, the patch breaks few reflection based tests that relay on > binary modifiers values.
I am not sure I understand what the effect is here - does it allow any measurable improvement? If not, I'd wait until the after the branch. But I could change my mind if there's real performance gain... The test part, however, which replaces bits with constants, I think is good anytime, I don't think we should have been using bit values anyway.
-- Stas Malyshev smalyshev@gmail.com

Sara Golemon

8 years ago
On Wed, Jul 25, 2018 at 11:01 AM, Dmitry Stogov <dmitry@zend.com> wrote:
> > I tried to fix ZEND_ACC_* flags mess. > > https://gist.github.com/dstogov/3b6ae377c17524b219670960cf98f8c1 > > > The patch specifies flags meaning, and reorder them according to meaning and frequency of usage (this allows generation of shorter instructions on x86). > Unfortunately, the patch breaks few reflection based tests that relay on binary modifiers values. >
Documentation part: LOVE 💖 LOVE 💖 LOVE Renumbering part: Eh, could save that for 8.0, but I won't complain about it in 7.4. Whichever version it lands in, please make a bump to ZEND_MODULE_API_NO part of the commit, and a comment in UPGRADING.INTERNALS as well. -Sara