Do not use property metadata to get/set object values

[goetas]

Q	A
Bug fix?	no
New feature?	no
Doc updated	no
BC breaks?	yes
Deprecations?	no
Tests pass?	yes
Fixed tickets	#932
License	Apache-2.0

first draft of what could be a solution for #932. To get some extra memory improvements, even jms/metadata should be updated (removing the reflection stuff from property metadata)

[Majkl578]

Nice. This bunch of IFs calls for a strategy pattern imho, this is a bit too coupled.

[goetas]

this has to be fast! very fast! 🤔
not sure if having extra classes to load and call will influence on it

[Majkl578]

Inho one shared accessor is enough, no? They are bound to type only.

[goetas]

The body of the closure is different,

function ($o, $name, $value) {
   $o->$name = $value;
}

vs

function ($o, $name) {
   return $o->$name;
}

so cant be done

[Majkl578]

As an experiment (perf), you could also try closures bound to specific object instances and/or properties.

[goetas]

As an experiment (perf), you could also try closures bound to specific object instances and/or properties.

Can you explain?

[Majkl578]

Right now you are using unbound accessors (2nd argument to Closure::bind() is null) so you have to pass around $o as an object instance.
If you bind the closure to specific object, you can simply use $this->$propertyName instead.

[Majkl578]

Btw this needs some test cases for inheritance with private properties: with different names, with same names (but different serialized names) and so.

[goetas]

Right now you are using unbound accessors (2nd argument to Closure::bind() is null) so you have to pass around $o as an object instance.
If you bind the closure to specific object, you can simply use $this->$propertyName instead.

But if i do it per object is much slower... creating the closure takes a lot apparently.

[goetas]

Btw this needs some test cases for inheritance with private properties: with different names, with same names (but different serialized names) and so.

The whole testsuite is using this access strategy, so should work, no?

[Majkl578]

Not sure. Your accessor is bound to specific class name which is crucial for private properties.
Assume this code:

class Foo
{
    /** @Serializer\SerializedName("foo") */
    private $a;
}

class Bar extends Foo
{
    /** @Serializer\SerializedName("bar") */
    private $a;
}

Will that work when $o instanceof Bar? (Or actually does that work now? 😆 )

[goetas]

#935 proves that does not work :)