[RFC] Extract PropertyAccessor readProperty and writeProperty methods to allow faster ObjectNormalizer

[fbourigault]

Background

While working on improving the ObjectNormalizer, I spotted that most of the execution time is spent in PropertyAccessor::getValue (https://blackfire.io/profiles/e52b7b58-060d-448a-bcc4-015df290812a/graph).

After looking at the PropertyAccessor::getValue, I first had the feeling that calling PropertyAccessor::readProperiesUntil to get an attribute value was overkill.
After adding a PropertyAccessor::hack to expose PropertyAccessor::readProperty, I got a significant improvement of ObjectNormalizer perfs (https://blackfire.io/profiles/compare/6e4aa9d9-c5dd-4243-89c0-c19f431b6bc9/graph).

Proposal

The idea is to extract PropertyAccessor::readProperty to allow it's usage by the ObjectNormalizer and bypass all the array logic executed when calling PropertyAccessor::getValue.

To achieve this, I would first replace the $zval and $result arrays by an internal Zval class. This will allow type safety.

/**
 * @internal
 */
class Zval
{
    public $value;
    public $ref;
    public $isRefChained;
}

Then I would add two internal classes (ObjectPropertyReader and ObjectPropertyWriter) that will use Zval objects as input and output.

/**
 * @internal
 */
class ObjectPropertyReader
{
    /**
     * Reads the a property from an object.
     *
     * @param Zval   $zval     The array containing the object to read from
     * @param string $property The property to read
     *
     * @return Zval The array containing the value of the property
     *
     * @throws NoSuchPropertyException if the property does not exist or is not public
     */
    public function readProperty(Zval $zval, string $property): Zval
    {
        // current PropertyAccessor::readProperty code modified to use Zval instead of array.
    }
}

/**
 * @internal
 */
class ObjectPropertyWriter
{
    /**
     * Sets the value of a property in the given object.
     *
     * @param Zval   $zval     The array containing the object to write to
     * @param string $property The property to write
     * @param mixed  $value    The value to write
     *
     * @throws NoSuchPropertyException if the property does not exist or is not public
     */
    public function writeProperty(Zval $zval, string $property, $value): void
    {
        // current PropertyAccessor::writeProperty code modified to use Zval instead of array.
    }
}

Finally I would add an ObjectPropertyAccessor class that will expose ObjectPropertyReader::readProperty and ObjectPropertyWriter::writeProperty to users and hide Zval stuff.

class ObjectPropertyAccessor implements ObjectPropertyAccessorInterface
{
    private $reader;
    private $writer;

    public function __construct(ObjectPropertyReader $reader, ObjectPropertyWriter $writer)
    {
        $this->reader = $reader;
        $this->writer = $writer;
    }

    public function readProperty($object, string $property)
    {
        $zval = new Zval();  
        $zval->value = $object;  
  
        return $this->reader->readProperty($zval, $property)->value;
    }

    public function writeProperty($object, string $property, $value)
    {
        $zval = new Zval();  
        $zval->value = $object;  
        $zval->ref = &$object;
        $this->writer->writeProperty($zval, $property, $value);
    }
}

The ObjectNormalizer will use this new ObjectPropertyAccessor instead of the PropertyAccessor to get the perf improvement.

Going further

Deprecating PropertyNormalizer and GetSetMethodNormalizer

Having an interface to access object properties will help us to deprecate PropertyNormalizer and GetSetMethodNormalizer by extracting code used to access properties into implementations of ObjectPropertyAccessorInterface.

Extracting getReadAccessInfo and getWriteAccessInfo from ObjectPropertyAccessor

To go further in the performance improvement, we could extract the getReadAccessInfo and getWriteAccessInfo logic to an other class. Then we will be able to ship a decorator which leverage the PHP7 static array cache.

Having an interface for getReadAccessInfo and getWriteAccessInfo open the path to solve issues like #9336 by introducing metadata which could also be cached to keep the performance!

WDYT ?

cc @dunglas @nicolas-grekas, @Tobion and @bendavies.

[dunglas]

I 100% agree with this agenda.

[fbourigault]

@nicolas-grekas any thoughts on this?

[bendavies]

probably related:
#22051

[fbourigault]

Closing as #29999 is a better solution.