Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:126045
Feedback-ID: ifab94697:Fastmail
Precedence: bulk
MIME-Version: 1.0
Date: Sun, 24 Nov 2024 12:49:44 +0100
To: internals@lists.php.net
Message-ID: <1fbc115a-dc22-4997-84cb-0ed7de2990dc@app.fastmail.com>
In-Reply-To: 
 <CAPyj-LCvQGOgLoLAZWOUo7UGTnnVCmK6MNNQPpqUBY7dkS9uyg@mail.gmail.com>
References: <18b85ba5-5f1c-489c-9096-3ae203977fbe@app.fastmail.com>
 <CAPyj-LCvQGOgLoLAZWOUo7UGTnnVCmK6MNNQPpqUBY7dkS9uyg@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] Data Classes
Content-Type: multipart/alternative;
 boundary=ae168ccaf1ad4b60a2d588d5eba20c11
From: rob@bottled.codes ("Rob Landers")

--ae168ccaf1ad4b60a2d588d5eba20c11
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

On Sat, Nov 23, 2024, at 23:10, Ilija Tovilo wrote:
> Hi Rob
>=20
> On Sat, Nov 23, 2024 at 2:12=E2=80=AFPM Rob Landers <rob@bottled.codes=
> wrote:
> >
> > Born from the Records RFC (https://wiki.php.net/rfc/records) discuss=
ion, I would like to introduce to you a competing RFC: Data Classes (htt=
ps://wiki.php.net/rfc/dataclass).
>=20
> As others have pointed out, your RFC is very similar to my proposal
> for struct. I don't quite understand the reason to compete and race
> each other to the finish line. Combined efforts are usually better.

This isn't a race or competition, and I meant "competing with records" a=
nd not "competing with Ilija." Though, I see how it could be interpreted=
 that way.

To be honest, I don't like this RFC. I think records and/or structs are =
the right answer (dedicated behavior vs. trying to stuff more behavior i=
nto classes). As mentioned elsewhere in the thread, the behavior here is=
 more of an emergent property than any actual thinking through it. So, t=
he fact that it is similar to your proposal is merely coincidental; it w=
asn't planned that way (nor did I catch it before submitting the RFC).=20

This mailing list is the only real interaction I have with other php-src=
 devs (and the occasional PR to php-src), so please forgive me if I don'=
t notice something. Yes, that isn't a real excuse, but not having other =
people to bounce ideas off of usually means I'll make an idiot out of my=
self, eventually.

That being said, there are a ton of edge cases I've uncovered where ever=
ything breaks down in this RFC:
 =E2=80=A2 PHP references are "evil", so I've discovered.
 =E2=80=A2 "new" generates some strange opcodes=E2=80=94I'm currently in=
vestigating this=E2=80=94that make dealing with value types difficult. I=
'm trying to solve this in some way that doesn't require changing the ge=
nerated opcodes, but that might be impossible. This solution would allow=
 using "new" with records and solve the constructor problem in this RFC.
 =E2=80=A2 Did I mention PHP references are evil?
In any case, I'd much rather help with your structs proposal, as the mor=
e I work on this, the more I don't like it.

>=20
> One of the bigger differences between our proposals is the addition of
> mutating methods in my proposal compared to yours. You show the
> following example in your RFC:
>=20
> ```php
> data class Rectangle {
>     public function __construct(public int $width, public int $height)=
 {}
>=20
>     public function resize(int $width, int $height): static {
>         $this->height =3D $height;
>         $this->width =3D $width;
>         return $this;
>     }
> }
> ```
>=20
> The resize method here modifies the instance and thus implicitly
> creates a copy. That's _fine_ for such a small structure. However,
> note that this still leads to the performance issues we have
> previously discussed for growable data structures.
>=20
> ```php
> data class Vector {
>     public function append(mixed $value): static {
>         /* Internal implementation, $values is some underlying storage=
. */
>         $this->values[] =3D $value;
>         return $this;
>     }
> }
> ```
>=20
> Calling `$vector->append(42);` will increase the refcount of
> `$vector`, and cause separation on `$this->values[] =3D ...;`. If
> `$vector->values` is a big storage, cloning will be very expensive.
> Hence, appending becomes an O(n) operation (because each element in
> the vector is copied to the new structure), and hence appending to an
> array in a loop will tank your performance.  That's the reason for the
> introduction of the `$vector->append!(42)` syntax in my proposal. It
> separates the value at call-site when necessary, and avoids separation
> on `$this` in methods altogether.

As I mentioned to Larry in the records discussion, the biggest problem w=
ith a "data class" is that it really needs dedicated syntax to be done p=
roperly (such as the ones in structs, and I plan to remove some syntax f=
eatures from records since I got a lot of negative feedback about the sy=
ntax there; shout out to reddit). I don't think that would belong to a g=
eneral class but rather a dedicated type. There really isn't a "one size=
 fits all" solution here.

>=20
> There might be some general confusion on the performance issue. In one
> of your e-mails in the last thread, you have mentioned:
>=20
> > Like Ilija mentioned in their email, there are significant performan=
ce optimizations to be had here that are simply not possible using regul=
ar (readonly) classes. I didn't go into detail as to how it works becaus=
e it feels like an implementation detail, but I will spend some time dis=
tilling this and its consequences, into the RFC, over the coming days. A=
s a simple illustration, there can be significant memory usage improveme=
nts:
> >
> > 100,000 arrays: https://3v4l.org/Z4CcV
> > 100,000 readonly classes: https://3v4l.org/1vhNp
>=20
> First off, the array example only uses less memory because [1, 2] is a
> constant array. When you make it dynamic, they will become way less
> efficient than objects. https://3v4l.org/pETM9
>=20
> But this is not the point I was trying to make either. Rather, when it
> comes to immutable, growable data structures, every mutation becomes
> an extremely expensive operation because the entire data structure,
> including its underlying storage, needs to be copied. For example:
>=20
> https://3v4l.org/BEsYT
>=20
> ```php
> class Vector {
>     private $values;
>=20
>     public function populate() {
>         $this->values =3D range(1, 1_000_000);
>     }
>=20
>     public function appendMutable() {
>         $this->values[] =3D 100_000_001;
>     }
>=20
>     public function appendImmutable() {
>         $new =3D clone $this;
>         $this->values[] =3D 100_000_001;
>     }
> }
> ```
>=20
> > appendMutable(): float(8.106231689453125E-6)
> > appendImmutable(): float(0.012187957763671875)
>=20
> That's a factor of 1 500 difference for an array containing 1 million
> numbers. Obviously, concrete numbers will vary, but the problem grows
> the bigger the array becomes.
>=20
> Ilija
>=20

I'm not quite focused on actual performance (yet), but I understand your=
 point. To that, I have an idea I've been playing around with to have "l=
ayered hashmaps" where a copy/clone is just a layer on the original (mad=
e immutable) hashmap, thus having a near zero cost for copies. There's s=
till extra indirection involved, so it potentially could be worse =F0=9F=
=A4=B7. I would think an average case O(1) would be faster than a copy, =
at least for large maps, but wall-clock time and Big O don't always corr=
elate. I also note that the benchmarks in the repo are based on operatio=
ns, not time, so it is quite difficult to show that a feature that incre=
ases the number of operations decreases the overall time. In other words=
, you may add 100 operations with a strong cache locality and remove 10 =
with poor cache locality -- quality vs. quantity, an age-old problem. An=
yway, there's no way to tell the exact performance characteristics until=
 I finish the implementation. I'd love to discuss this more, as it's act=
ually pretty neat and interesting to solve in a performant way.

It could also be like my zend_string refactor I spent a few months on ov=
er the summer, where the performance isn't affected (at least for wall-c=
lock time), and the only added benefits showing when you perform lots of=
 string modifications. Other benefits won't make sense without already h=
aving an RFC I was working on (that is still in the draft stage). So, fo=
r now, that branch will just gather dust.

Things like this are largely why I didn't put too much information about=
 the performance characteristics of records in my RFC. It's really an im=
plementation detail, and I didn't want people's decisions to be tied to =
an implementation detail that could change drastically. In other words, =
maybe the performance would be poor now, but I'm confident it can be imp=
roved later. IMHO, we should focus on whether we want the feature in the=
 first place rather than worrying about how fast or slow it is. Many imp=
rovements don't make sense for the sake of improving them, and it isn't =
until there is a problem to be solved (such as a poor performing feature=
) that the improvement becomes worthwhile.

=E2=80=94 Rob
--ae168ccaf1ad4b60a2d588d5eba20c11
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: quoted-printable

<!DOCTYPE html><html><head><title></title><style type=3D"text/css">p.Mso=
Normal,p.MsoNoSpacing{margin:0}</style></head><body><div>On Sat, Nov 23,=
 2024, at 23:10, Ilija Tovilo wrote:<br></div><blockquote type=3D"cite" =
id=3D"qt" style=3D""><div>Hi Rob<br></div><div><br></div><div>On Sat, No=
v 23, 2024 at 2:12=E2=80=AFPM Rob Landers &lt;<a href=3D"mailto:rob@bott=
led.codes">rob@bottled.codes</a>&gt; wrote:<br></div><div>&gt;<br></div>=
<div>&gt; Born from the Records RFC (<a href=3D"https://wiki.php.net/rfc=
/records">https://wiki.php.net/rfc/records</a>) discussion, I would like=
 to introduce to you a competing RFC: Data Classes (<a href=3D"https://w=
iki.php.net/rfc/dataclass">https://wiki.php.net/rfc/dataclass</a>).<br><=
/div><div><br></div><div>As others have pointed out, your RFC is very si=
milar to my proposal<br></div><div>for struct. I don't quite understand =
the reason to compete and race<br></div><div>each other to the finish li=
ne. Combined efforts are usually better.<br></div></blockquote><div><br>=
</div><div>This isn't a race or competition, and I meant "competing with=
 records" and not "competing with&nbsp;Ilija." Though, I see how it coul=
d be interpreted that way.<br></div><div><br></div><div>To be honest, I =
don't like this RFC. I think records and/or structs are the right answer=
 (dedicated behavior vs. trying to stuff more behavior into classes). As=
 mentioned elsewhere in the thread, the behavior here is more of an emer=
gent property than any actual thinking through it. So, the fact that it =
is similar to your proposal is merely coincidental; it wasn't planned th=
at way (nor did I catch it before submitting the RFC). <br></div><div><b=
r></div><div>This mailing list is the only real interaction I have with =
other php-src devs (and the occasional PR to php-src), so please forgive=
 me if I don't notice something. Yes, that isn't a real excuse, but not =
having other people to bounce ideas off of usually means I'll make an id=
iot out of myself, eventually.<br></div><div><br></div><div>That being s=
aid, there are a ton of edge cases I've uncovered where everything break=
s down in this RFC:<br></div><ul><li>PHP references are "evil", so I've =
discovered.<br></li><li>"new" generates some strange opcodes=E2=80=94I'm=
 currently investigating this=E2=80=94that make dealing with value types=
 difficult. I'm trying to solve this in some way that doesn't require ch=
anging the generated opcodes, but that might be impossible. This solutio=
n would allow using "new" with records and solve the constructor problem=
 in this RFC.<br></li><li>Did I mention PHP references are evil?<br></li=
></ul><div>In any case, I'd much rather help with your structs proposal,=
 as the more I work on this, the more I don't like it.</div><div><br></d=
iv><blockquote type=3D"cite" id=3D"qt" style=3D""><div><br></div><div>On=
e of the bigger differences between our proposals is the addition of<br>=
</div><div>mutating methods in my proposal compared to yours. You show t=
he<br></div><div>following example in your RFC:<br></div><div><br></div>=
<div>```php<br></div><div>data class Rectangle {<br></div><div>&nbsp;&nb=
sp;&nbsp; public function __construct(public int $width, public int $hei=
ght) {}<br></div><div><br></div><div>&nbsp;&nbsp;&nbsp; public function =
resize(int $width, int $height): static {<br></div><div>&nbsp;&nbsp;&nbs=
p;&nbsp;&nbsp;&nbsp;&nbsp; $this-&gt;height =3D $height;<br></div><div>&=
nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; $this-&gt;width =3D $width;<br=
></div><div>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; return $this;<br>=
</div><div>&nbsp;&nbsp;&nbsp; }<br></div><div>}<br></div><div>```<br></d=
iv><div><br></div><div>The resize method here modifies the instance and =
thus implicitly<br></div><div>creates a copy. That's _fine_ for such a s=
mall structure. However,<br></div><div>note that this still leads to the=
 performance issues we have<br></div><div>previously discussed for growa=
ble data structures.<br></div><div><br></div><div>```php<br></div><div>d=
ata class Vector {<br></div><div>&nbsp;&nbsp;&nbsp; public function appe=
nd(mixed $value): static {<br></div><div>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&=
nbsp;&nbsp; /* Internal implementation, $values is some underlying stora=
ge. */<br></div><div>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; $this-&g=
t;values[] =3D $value;<br></div><div>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp=
;&nbsp; return $this;<br></div><div>&nbsp;&nbsp;&nbsp; }<br></div><div>}=
<br></div><div>```<br></div><div><br></div><div>Calling `$vector-&gt;app=
end(42);` will increase the refcount of<br></div><div>`$vector`, and cau=
se separation on `$this-&gt;values[] =3D ...;`. If<br></div><div>`$vecto=
r-&gt;values` is a big storage, cloning will be very expensive.<br></div=
><div>Hence, appending becomes an O(n) operation (because each element i=
n<br></div><div>the vector is copied to the new structure), and hence ap=
pending to an<br></div><div>array in a loop will tank your performance.&=
nbsp; That's the reason for the<br></div><div>introduction of the `$vect=
or-&gt;append!(42)` syntax in my proposal. It<br></div><div>separates th=
e value at call-site when necessary, and avoids separation<br></div><div=
>on `$this` in methods altogether.<br></div></blockquote><div><br></div>=
<div>As I mentioned to Larry in the records discussion, the biggest prob=
lem with a "data class" is that it really needs dedicated syntax to be d=
one properly (such as the ones in structs, and I plan to remove some syn=
tax features from records since I got a lot of negative feedback about t=
he syntax there; shout out to reddit). I don't think that would belong t=
o a general class but rather a dedicated type. There really isn't a "one=
 size fits all" solution here.<br></div><div><br></div><blockquote type=3D=
"cite" id=3D"qt" style=3D""><div><br></div><div>There might be some gene=
ral confusion on the performance issue. In one<br></div><div>of your e-m=
ails in the last thread, you have mentioned:<br></div><div><br></div><di=
v>&gt; Like Ilija mentioned in their email, there are significant perfor=
mance optimizations to be had here that are simply not possible using re=
gular (readonly) classes. I didn't go into detail as to how it works bec=
ause it feels like an implementation detail, but I will spend some time =
distilling this and its consequences, into the RFC, over the coming days=
. As a simple illustration, there can be significant memory usage improv=
ements:<br></div><div>&gt;<br></div><div>&gt; 100,000 arrays:&nbsp;<a hr=
ef=3D"https://3v4l.org/Z4CcV">https://3v4l.org/Z4CcV</a><br></div><div>&=
gt; 100,000 readonly classes:&nbsp;<a href=3D"https://3v4l.org/1vhNp">ht=
tps://3v4l.org/1vhNp</a><br></div><div><br></div><div>First off, the arr=
ay example only uses less memory because [1, 2] is a<br></div><div>const=
ant array. When you make it dynamic, they will become way less<br></div>=
<div>efficient than objects.&nbsp;<a href=3D"https://3v4l.org/pETM9">htt=
ps://3v4l.org/pETM9</a><br></div><div><br></div><div>But this is not the=
 point I was trying to make either. Rather, when it<br></div><div>comes =
to immutable, growable data structures, every mutation becomes<br></div>=
<div>an extremely expensive operation because the entire data structure,=
<br></div><div>including its underlying storage, needs to be copied. For=
 example:<br></div><div><br></div><div><a href=3D"https://3v4l.org/BEsYT=
">https://3v4l.org/BEsYT</a><br></div><div><br></div><div>```php<br></di=
v><div>class Vector {<br></div><div>&nbsp;&nbsp;&nbsp; private $values;<=
br></div><div><br></div><div>&nbsp;&nbsp;&nbsp; public function populate=
() {<br></div><div>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; $this-&gt;=
values =3D range(1, 1_000_000);<br></div><div>&nbsp;&nbsp;&nbsp; }<br></=
div><div><br></div><div>&nbsp;&nbsp;&nbsp; public function appendMutable=
() {<br></div><div>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; $this-&gt;=
values[] =3D 100_000_001;<br></div><div>&nbsp;&nbsp;&nbsp; }<br></div><d=
iv><br></div><div>&nbsp;&nbsp;&nbsp; public function appendImmutable() {=
<br></div><div>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; $new =3D clone=
 $this;<br></div><div>&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; $this-&=
gt;values[] =3D 100_000_001;<br></div><div>&nbsp;&nbsp;&nbsp; }<br></div=
><div>}<br></div><div>```<br></div><div><br></div><div>&gt; appendMutabl=
e(): float(8.106231689453125E-6)<br></div><div>&gt; appendImmutable(): f=
loat(0.012187957763671875)<br></div><div><br></div><div>That's a factor =
of 1 500 difference for an array containing 1 million<br></div><div>numb=
ers. Obviously, concrete numbers will vary, but the problem grows<br></d=
iv><div>the bigger the array becomes.<br></div><div><br></div><div>Ilija=
<br></div><div><br></div></blockquote><div><br></div><div>I'm not quite =
focused on actual performance (yet), but I understand your point. To tha=
t, I have an idea I've been playing around with to have "layered hashmap=
s" where a copy/clone is just a layer on the original (made immutable) h=
ashmap, thus having a near zero cost for copies. There's still extra ind=
irection involved, so it potentially could be worse =F0=9F=A4=B7. I woul=
d think an average case O(1) would be faster than a copy, at least for l=
arge maps, but wall-clock time and Big O don't always correlate. I also =
note that the benchmarks in the repo are based on operations, not time, =
so it is quite difficult to show that a feature that increases the numbe=
r of operations decreases the overall time. In other words, you may add =
100 operations with a strong cache locality and remove 10 with poor cach=
e locality -- quality vs. quantity, an age-old problem. Anyway, there's =
no way to tell the exact performance characteristics until I finish the =
implementation. I'd love to discuss this more, as it's actually pretty n=
eat and interesting to solve in a performant way.<br></div><div><br></di=
v><div>It could also be like my zend_string refactor I spent a few month=
s on over the summer, where the performance isn't affected (at least for=
 wall-clock time), and the only added benefits showing when you perform =
lots of string modifications. Other benefits won't make sense without al=
ready having an RFC I was working on (that is still in the draft stage).=
 So, for now, that branch will just gather dust.<br></div><div><br></div=
><div>Things like this are largely why I didn't put too much information=
 about the performance characteristics of records in my RFC. It's really=
 an implementation detail, and I didn't want people's decisions to be ti=
ed to an implementation detail that could change drastically. In other w=
ords, maybe the performance would be poor now, but I'm confident it can =
be improved later. IMHO, we should focus on whether we want the feature =
in the first place rather than worrying about how fast or slow it is. Ma=
ny improvements don't make sense for the sake of improving them, and it =
isn't until there is a problem to be solved (such as a poor performing f=
eature) that the improvement becomes worthwhile.<br></div><div><br></div=
><div id=3D"sig121229152">=E2=80=94 Rob<br></div></body></html>
--ae168ccaf1ad4b60a2d588d5eba20c11--