Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:102915
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain zend.com from 104.47.33.130 cause and error)
To: Zeev Suraski <vsuraski@gmail.com>, Nikita Popov <nikita.ppv@gmail.com>
CC: Zeev Suraski <zeev@zend.com>, Internals <internals@lists.php.net>
Thread-Topic: [PHP-DEV] Re: [RFC] Typed Properties
Thread-Index: AQHUHT2gcpsR0kfTKkGYW25pPRMBNaSSaTaAgAFM2YCAAoskgIAAKreP
Date: Thu, 19 Jul 2018 11:33:08 +0000
Message-ID:
 <BY1PR02MB126021361704BA68D5F143A7BF520@BY1PR02MB1260.namprd02.prod.outlook.com>
References:
 <CAF+90c_L7S+EB8YxdNNpNZ7NhhYwCbwEw5_TSnSwUNN-zLVvCg@mail.gmail.com>
 <CAF+90c8ZG8FYy74i+HmGPb=YgsaT=CLRDqs0TxNo+X13AeyeTg@mail.gmail.com>
 <BL0PR02MB443503A3D9340A7A91E8AD5DAC5D0@BL0PR02MB4435.namprd02.prod.outlook.com>
 <CAF+90c9o3ou_QGGMAV=S6NeXeTB+-ew=um=+ZJocQWDhgvHDNw@mail.gmail.com>,<CAJHtznxDSYGuUoi3G_C=WsDtiShPV64Pjj_cvgpX82=7QvbOKQ@mail.gmail.com>
In-Reply-To:
 <CAJHtznxDSYGuUoi3G_C=WsDtiShPV64Pjj_cvgpX82=7QvbOKQ@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: zend.com does not designate
 permitted sender hosts)
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: multipart/alternative;
	boundary="_000_BY1PR02MB126021361704BA68D5F143A7BF520BY1PR02MB1260namp_"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 05799c6b-d89c-45ba-a279-08d5ed6b6700
X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Jul 2018 11:33:08.6900
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 32210298-c08b-4829-8097-6b12c025a892
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY1PR02MB1210
Subject: Re: [PHP-DEV] Re: [RFC] Typed Properties
From: dmitry@zend.com (Dmitry Stogov)

--_000_BY1PR02MB126021361704BA68D5F143A7BF520BY1PR02MB1260namp_
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable

I've run few benchmarks, to measure the performance penalty of this proposa=
l.


https://gist.github.com/dstogov/b9fc0fdccfb8bf7bae121ce3d3ff1db1


In most cases real-life apps become ~1% slower. In the worst case, I got 6%=
 slowdown (on mediawiki).


Thanks. Dmitry.

________________________________
From: Zeev Suraski <vsuraski@gmail.com>
Sent: Thursday, July 19, 2018 11:52:37 AM
To: Nikita Popov
Cc: Zeev Suraski; Internals
Subject: Re: [PHP-DEV] Re: [RFC] Typed Properties

On Tue, Jul 17, 2018 at 9:02 PM Nikita Popov <nikita.ppv@gmail.com> wrote:

>
> Sure, we can wait another week. Unless the additional discussion results =
in
> major changes to the RFC, we'll start voting next Monday (2018-07-23). In
> the interest of avoiding further delays, please try to view this as a har=
d
> deadline: If you would like to discuss some aspect of the proposal or rai=
se
> a concern, please do so now rather than on Monday morning.
>

I reviewed the proposal in detail - and quite detailed it is.  It seems
like you and Bob did a very thorough job here, kudos on that.

I do have 3 comments - none of them major - but I think should be addressed
nonetheless:

1. The RFC is surprisingly sparse on examples where the typed properties
are not scalar ones.  So much, in fact, that I had to check whether this is
intentional (i.e., classes are unsupported as enforced property types) or
an oversight.  Reviewing the 'Supported Types' section, while mentioning a
ClassOrInterface - isn't entirely clear, as it's unclear whether this is a
token, or something that stands for any class name or interface.
Thankfully the proposal does cover class and interface names as property
type definitions.  I would recommend to be explicit here, and say "any
class or interface name", and ideally also provide a sample of that option
in the Introduction section of the RFC.
2.  While trying to understand whether #1 was in fact supported and
reviewing the patch, I noticed that there were several comments from Dmitry
on the patch (within the pull request).  It would be nice if they were
addressed (none of them will turn the patch upside down, and as I see it
they are orthogonal to the vote).
3. The patch does bring a performance penalty, albeit quite small (every
property assignment now has to conduct type safety checks).  This does not
only affect code that uses typed properties - but also code that doesn't.
I think this should be mentioned in the RFC.  To be clear - I don't think
the penalty is substantial enough to change someone's opinion about the
merits of the proposal one way or another - but in the spirit of full
disclosure it should be there.

Zeev

--_000_BY1PR02MB126021361704BA68D5F143A7BF520BY1PR02MB1260namp_--