[RFC][DISCUSSION] Immutability

php.internals

Silvio Marijić

8 years ago
Hi, I am starting this discussion again after a year since this RFC was created. From discussions with people in the community some suggestions came up regarding issues that were main reason that development on this RFC stopped. RFC is updated along with implementation (link at the bottom of RFC). I'll be more then happy to answer all questions.
-- Silvio Marijić Software Engineer SMSGlobal

Nikita Popov

8 years ago
On Tue, Feb 20, 2018 at 12:20 PM, Silvio Marijić <marijic.silvio@gmail.com> wrote:
> Hi, > I am starting this discussion again after a year since this RFC was > created. From discussions with people in the community some suggestions > came up regarding issues that were main reason that development on this RFC > stopped. RFC is updated along with implementation (link at the bottom of > RFC). I'll be more then happy to answer all questions.
Link: https://wiki.php.net/rfc/immutability :) Nikita

Rowan Collins

8 years ago
On 20 February 2018 at 11:25, Nikita Popov <nikita.ppv@gmail.com> wrote:
> On Tue, Feb 20, 2018 at 12:20 PM, Silvio Marijić <marijic.silvio@gmail.com > > > wrote: > > > Hi, > > I am starting this discussion again after a year since this RFC was > > created. From discussions with people in the community some suggestions > > came up regarding issues that were main reason that development on this > RFC > > stopped. RFC is updated along with implementation (link at the bottom of > > RFC). I'll be more then happy to answer all questions. > > > Link: https://wiki.php.net/rfc/immutability :)
This definitely seems like an exciting addition to the language. Some clarifications: This sentence in the RFC is rather confusing, "Any references to objects ... cannot be references to scalars ... or may be ...". I think it's just reiterating things said elsewhere in the RFC, but could maybe be reworded?
> Any references to objects passed into an immutable class constructor
cannot be references to scalars or may be immutable class instances. A set of bullet points of what types can and can't be assigned to an immutable property might be useful. The example under "references" shows a reference being successfully taken to an immutable property, but then being considered immutable itself. Is there any performance penalty to tracking this extra state? I know references have caused problems in other cases, like the typed properties RFC. What kind of error is produced when you attempt to write to an immutable property? I would expect some sub-class of Error to be thrown, but the RFC currently just says a fatal error. As I say, I like the idea, and thanks for working on it. Regards,
-- Rowan Collins [IMSoP]

Silvio Marijić

8 years ago
Hi Rowan, I agree that RFC needs a bit refinement which I hope to do during ongoing discussion here. Originally you couldn't assign following types to an immutable property - *Resource* - *Instance of none-immutable class * Now I had to resort to also add *array* to that list. Regarding your question about references, yes you can obtain reference to an immutable property but you can not modify that property by reference. In that case it will throw fatal error, but I agree that it would be better to throw exception, I will include that in RFC. Thanks for the support I really appreciate it. Regards, 2018-02-22 10:57 GMT+01:00 Rowan Collins <rowan.collins@gmail.com>:
> On 20 February 2018 at 11:25, Nikita Popov <nikita.ppv@gmail.com> wrote: > > > On Tue, Feb 20, 2018 at 12:20 PM, Silvio Marijić < > marijic.silvio@gmail.com > > > > > wrote: > > > > > Hi, > > > I am starting this discussion again after a year since this RFC was > > > created. From discussions with people in the community some suggestions > > > came up regarding issues that were main reason that development on this > > RFC > > > stopped. RFC is updated along with implementation (link at the bottom > of > > > RFC). I'll be more then happy to answer all questions. > > > > > > Link: https://wiki.php.net/rfc/immutability :) > > > > This definitely seems like an exciting addition to the language. Some > clarifications: > > > This sentence in the RFC is rather confusing, "Any references to objects > ... cannot be references to scalars ... or may be ...". I think it's just > reiterating things said elsewhere in the RFC, but could maybe be reworded? > > > Any references to objects passed into an immutable class constructor > cannot be references to scalars or may be immutable class instances. > > A set of bullet points of what types can and can't be assigned to an > immutable property might be useful. > > > The example under "references" shows a reference being successfully taken > to an immutable property, but then being considered immutable itself. Is > there any performance penalty to tracking this extra state? I know > references have caused problems in other cases, like the typed properties > RFC. > > > What kind of error is produced when you attempt to write to an immutable > property? I would expect some sub-class of Error to be thrown, but the RFC > currently just says a fatal error. > > As I say, I like the idea, and thanks for working on it. > > Regards, > -- > Rowan Collins > [IMSoP] >
-- Silvio Marijić Software Engineer SMSGlobal

Silvio Marijić

8 years ago
In addition to your question about impact on the performance, there is none. As there is no special tracking of those references just control check when you try to write by reference. 2018-02-23 14:40 GMT+01:00 Silvio Marijić <marijic.silvio@gmail.com>:
> Hi Rowan, > > I agree that RFC needs a bit refinement which I hope to do during ongoing > discussion here. > Originally you couldn't assign following types to an immutable property > > - *Resource* > - *Instance of none-immutable class * > > Now I had to resort to also add *array* to that list. > > > Regarding your question about references, yes you can obtain reference to > an immutable property but you can not modify that property by reference. In > that case it will throw fatal error, but I agree that it would be better to > throw exception, I will include that in RFC. > > Thanks for the support I really appreciate it. > > Regards, > > 2018-02-22 10:57 GMT+01:00 Rowan Collins <rowan.collins@gmail.com>: > >> On 20 February 2018 at 11:25, Nikita Popov <nikita.ppv@gmail.com> wrote: >> >> > On Tue, Feb 20, 2018 at 12:20 PM, Silvio Marijić < >> marijic.silvio@gmail.com >> > > >> > wrote: >> > >> > > Hi, >> > > I am starting this discussion again after a year since this RFC was >> > > created. From discussions with people in the community some >> suggestions >> > > came up regarding issues that were main reason that development on >> this >> > RFC >> > > stopped. RFC is updated along with implementation (link at the bottom >> of >> > > RFC). I'll be more then happy to answer all questions. >> > >> > >> > Link: https://wiki.php.net/rfc/immutability :) >> >> >> >> This definitely seems like an exciting addition to the language. Some >> clarifications: >> >> >> This sentence in the RFC is rather confusing, "Any references to objects >> ... cannot be references to scalars ... or may be ...". I think it's just >> reiterating things said elsewhere in the RFC, but could maybe be reworded? >> >> > Any references to objects passed into an immutable class constructor >> cannot be references to scalars or may be immutable class instances. >> >> A set of bullet points of what types can and can't be assigned to an >> immutable property might be useful. >> >> >> The example under "references" shows a reference being successfully taken >> to an immutable property, but then being considered immutable itself. Is >> there any performance penalty to tracking this extra state? I know >> references have caused problems in other cases, like the typed properties >> RFC. >> >> >> What kind of error is produced when you attempt to write to an immutable >> property? I would expect some sub-class of Error to be thrown, but the RFC >> currently just says a fatal error. >> >> As I say, I like the idea, and thanks for working on it. >> >> Regards, >> -- >> Rowan Collins >> [IMSoP] >> > > > > -- > Silvio Marijić > Software Engineer > SMSGlobal >
-- Silvio Marijić Software Engineer SMSGlobal

Paul M Jones

8 years ago
Hi Silvio, I like the RFC. * * * I would like to see arrays allowed as immutable properties. In <http://paul-m-jones.com/archives/6400> I wrote that arrays are "probably not practical in most situations" because you have to "recursively scan through array properties to make sure that they contain only immutable values". However, I think if the scanning happens at the C level it is not such a burden. John Boehr implemented it for the immutable ServerRequest object in ext-request <http://pecl.php.net/package/request> and it has worked well. If you do allow arrays, they would have to follow the same rules as the immutable object. Alternatively, perhaps an ImmutableArrayObject would be a good addition or followup to the RFC. * * * Some further points to harden the implementation: - I see that resources and references are disallowed (which is good). If you have not already done so, you may wish to disallow streams as well. - You might want to disable setting of undefined public properties, so that you cannot "add" mutable public properties to the immutable object. - Disable the constructor after it has been called, so the object cannot be "re-constructed" in place. * * * That's all I can think of for now. Thanks for putting this together.
-- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php

Silvio Marijić

8 years ago
Thanks Paul, I do have rough idea how to go about arrays possibly without recursive scan, I have to try couple of things, it could work if some checks are moved to zval level. For now it is in the future scope unless I manage to handle the within week or two. First two proposals are valid points. Regarding disabling constructor, that would fail as soon as you try to write properties in constructor since object is already "locked". Best regards and thanks for proposals. 2018-02-23 15:08 GMT+01:00 Paul Jones <pmjones88@gmail.com>:
> Hi Silvio, > > I like the RFC. > > * * * > > I would like to see arrays allowed as immutable properties. > > In <http://paul-m-jones.com/archives/6400> I wrote that arrays are > "probably not practical in most situations" because you have to > "recursively scan through array properties to make sure that they contain > only immutable values". > > However, I think if the scanning happens at the C level it is not such a > burden. John Boehr implemented it for the immutable ServerRequest object in > ext-request <http://pecl.php.net/package/request> and it has worked well. > > If you do allow arrays, they would have to follow the same rules as the > immutable object. > > Alternatively, perhaps an ImmutableArrayObject would be a good addition or > followup to the RFC. > > * * * > > Some further points to harden the implementation: > > - I see that resources and references are disallowed (which is good). If > you have not already done so, you may wish to disallow streams as well. > > - You might want to disable setting of undefined public properties, so > that you cannot "add" mutable public properties to the immutable object. > > - Disable the constructor after it has been called, so the object cannot > be "re-constructed" in place. > > * * * > > That's all I can think of for now. Thanks for putting this together. > > > -- > Paul M. Jones > pmjones@pmjones.io > http://paul-m-jones.com > > Modernizing Legacy Applications in PHP > https://leanpub.com/mlaphp > > Solving the N+1 Problem in PHP > https://leanpub.com/sn1php > > > >
-- Silvio Marijić Software Engineer SMSGlobal

Niklas Keller

8 years ago
Hey,
> I do have rough idea how to go about arrays possibly without recursive > scan, I have to try couple of things, it could work if some checks are > moved to zval level. For now it is in the future scope unless I manage to > handle the within week or two.
What's the issue with a one-time runtime check on assignment?
> First two proposals are valid points. Regarding disabling constructor, that > would fail as soon as you try to write properties in constructor since > object is already "locked".
You lock the object after the constructor has been called, not afterwards. If you don't do that, your objects won't be immutable. I like the RFC, but generally I think value objects that are passed by-value instead of by-reference are the better way to solve the problem. Regards, Niklas

Silvio Marijić

8 years ago
Hi Niklas, Problem with checking array on assignment is that it could be N dimensions deep and one would have perform check for each item in each dimension for mutable values which could have some performance implications. Regarding constructor call, upon first initialization of object, you are allowed to assign values to properties, after constructor is executed, object is locked. If you try to call again constructor on the same object it would fail as soon as execution reaches first assignment statement because object was already marked as locked. I agree that it would be better to pass by value but I am not sure how would that impact the way we currently manage objects in object store. Best regards, 2018-02-23 15:47 GMT+01:00 Niklas Keller <me@kelunik.com>:
> Hey, > > > I do have rough idea how to go about arrays possibly without recursive > > scan, I have to try couple of things, it could work if some checks are > > moved to zval level. For now it is in the future scope unless I manage to > > handle the within week or two. > > What's the issue with a one-time runtime check on assignment? > > > First two proposals are valid points. Regarding disabling constructor, > that > > would fail as soon as you try to write properties in constructor since > > object is already "locked". > > You lock the object after the constructor has been called, not > afterwards. If you don't do that, your objects won't be immutable. > > I like the RFC, but generally I think value objects that are passed > by-value instead of by-reference are the better way to solve the > problem. > > Regards, Niklas >
-- Silvio Marijić Software Engineer SMSGlobal

Niklas Keller

8 years ago
Hey,
> Problem with checking array on assignment is that it could be N dimensions > deep and one would have perform check for each item in each dimension for > mutable values which could have some performance implications.
if you want to enforce that in userland now, you have even higher performance implications. I wouldn't worry about the performance of edge cases and take away a feature for everybody else because of that. If there's a better internal implementation, it can always be improved later.
> Regarding constructor call, upon first initialization of object, you are > allowed to assign values to properties, after constructor is executed, > object is locked. If you try to call again constructor on the same object it > would fail as soon as execution reaches first assignment statement because > object was already marked as locked.
Sounds good! So it's already implemented basically.
> I agree that it would be better to pass by value but I am not sure how would > that impact the way we currently manage objects in object store.
If we both agree it would be the better way, why not draft an RFC for it and find someone who knows how to implement it? Regards, Niklas

Niklas Keller

8 years ago
Hey,
> If you do allow arrays, they would have to follow the same rules as the immutable object. > > Alternatively, perhaps an ImmutableArrayObject would be a good addition or followup to the RFC.
That would just move the problem and doesn't have any benefit over directly allowing arrays, right?
> Some further points to harden the implementation: > > - I see that resources and references are disallowed (which is good). If you have not already done so, you may wish to disallow streams as well.
Streams are resources, am I missing something? Regards, Niklas

Dan Ackroyd

8 years ago
On 23 February 2018 at 13:47, Silvio Marijić <marijic.silvio@gmail.com> wrote:
> In addition to your question about impact on the performance, there is > none.
Have you actually measured that there is no performance impact, or is that an assumption from what affect the code should have? cheers Dan