Generated hydrator and extractor should be a closure, not a class #64

Ocramius · 2017-01-14T01:21:30Z

As per discussions in #59, hydrator and extractor shouldn't be composed into a zend-hydrator, but should instead be two closures.

An example API would be:

interface GenerateHydrator 
{
    public function __invoke(string $className) : callable;
}

interface GenerateExtractor 
{
    public function __invoke(string $className) : callable;
}

They would be used as following:

$object = $generateHydrator->__invoke(get_class($object))($object, $data); 
$data = $generateExtractor->__invoke(get_class($object))($object);

Composing them into a Hydrator instance that follows zend-hydrator spec is simple, but we'd also get a decent performance improvement by just relying on functional composition above.

This would allow us to:

gain performance (one less method call per hydrate/extract)
remove the need of the visitor pattern to modify an existing class (we just generate closures from scratch)
simplify API
provide type-safe closures (since we can add parameter and return type hints)

The text was updated successfully, but these errors were encountered:

pounard · 2017-01-14T12:10:37Z

I have though of this, here are my ideas.

Nominal use case, no inheritance

class A {
    private $a;
}

Generated closure:

$hydrateA = Closure::bind(function (array $data, A $object) {
  $object->a = $values['a'];
}, null, A::class);

Hydration:

$object = new A();
$data = ['a' => 1];
$hydrateA($data, $object);

Notes:

class properties visibility does not matter

Inheritance with visible properties use case

class B {
    protected $b;
}

class A extends B {
    private $a;
}

Generated closure:

$hydrateA = Closure::bind(function (array $data, A $object) {
  $object->b = $values['b'];
  $object->a = $values['a'];
}, null, A::class);

Hydration:

$object = new A();
$data = ['a' => 1, 'b' => 2];
$hydrateA($data, $object);

Notes:

only one closure is needed, since properties are visible

Inheritance with hidden properties use case

class B {
    private $b;
}

class A extends B {
    private $a;
}

Generated closure:

$hydrateB = Closure::bind(function (array $data, B $object) {
  $object->b = $values['b'];
}, null, B::class);

$hydrateA = Closure::bind(function (array $data, A $object) use ($hydrateB) {
  $hydrateB->__invoke($data, $object);
  $object->a = $values['a'];
}, null, A::class);

Hydration:

$object = new A();
$data = ['a' => 1, 'b' => 2];
$hydrateA($data, $object);

Notes:

closure for A needs to use closure for B
caller does not need to know that closure for B exists
for closure (re)definition, defining all in the same scope if enough
for this to work

Deep inheritance with hidden properties use case

class C {
    private $c;
}

class B extends C {
    private $b;
}

class A extends B {
    private $a;
}

Generated closure:

$hydrateC = Closure::bind(function (array $data, C $object) {
  $object->c = $values['c'];
}, null, C::class);

$hydrateB = Closure::bind(function (array $data, B $object) use ($hydrateC) {
  $hydrateC->__invoke($data, $object);
  $object->b = $values['b'];
}, null, B::class);

$hydrateA = Closure::bind(function (array $data, A $object) use ($hydrateB) {
  $hydrateB->__invoke($data, $object);
  $object->a = $values['a'];
}, null, A::class);

Hydration:

$object = new A();
$data = ['a' => 1, 'b' => 2, 'c' => 3];
$hydrateA($data, $object);

Notes:

notes are the same as inheritance with hidden properties

Deep inheritance with mixed properties use case

class C {
    private $c;
}

class B extends C {
    protected $b;
}

class A extends B {
    private $a;
}

Generated closure:

$hydrateC = Closure::bind(function (array $data, C $object) {
  $object->c = $values['c'];
}, null, C::class);

$hydrateB = Closure::bind(function (array $data, B $object) use ($hydrateC) {
  $hydrateC->__invoke($data, $object);
  $object->b = $values['b'];
}, null, B::class);

$hydrateA = Closure::bind(function (array $data, A $object) use ($hydrateB) {
  $hydrateB->__invoke($data, $object);
  $object->a = $values['a'];
}, null, A::class);

Or alternate scenario where $hydrateB can be skipped:

$hydrateC = Closure::bind(function (array $data, C $object) {
  $object->c = $values['c'];
}, null, C::class);

$hydrateA = Closure::bind(function (array $data, A $object) use ($hydrateC) {
  $hydrateC->__invoke($data, $object);
  $object->b = $values['b'];
  $object->a = $values['a'];
}, null, A::class);

Hydration:

$object = new A();
$data = ['a' => 1, 'b' => 2];
$hydrateA($data, $object);

Notes:

closure for B hydration can be skipped
nevertheless, it can be generated as well even if unneeded: in case
hydrating a B instance is asked, it would be pre-generated

General notes

since closures are variables, using a custom namespace is enough for
avoiding name conflicts
all closures can and probably should be in the same namespace
all of this can be generated without using an AST, reflexion is enough
extractors works the same way

pounard · 2019-07-13T15:12:21Z

Actually I partially implemented this in a fork of mine. My use case here is to keep backward compatibility and zend-hydrator interface, so in order to achieve this:

instead of having an array access to get the extractor and hydrator callbacks, they are set on static properties,
static properties are initialized outside of the class, right under it within the same file.

Here is a an exemple:

/**
 * This is a generated hydrator for the MakinaCorpus\Normalizer\Benchmarks\MockTextWithFormat class
 */
final class G402f10984f89322030223b7630346c10ce9bf0a4 implements HydratorInterface
{
    public static $hydrate0;
    public static $extract0;

    public function hydrate(array $data, $object)
    {
        self::$hydrate0->__invoke($object, $data);
        return $object;
    }

    public function extract($object)
    {
        $ret = [];
        self::$extract0->__invoke($object, $ret);
        return $ret;
    }
}

G402f10984f89322030223b7630346c10ce9bf0a4::$hydrate0 = \Closure::bind(static function ($object, $values) {
    if (isset($values['text']) || $object->text !== null && \array_key_exists('text', $values)) {
        $object->text = $values['text'];
    }
    if (isset($values['format']) || $object->format !== null && \array_key_exists('format', $values)) {
        $object->format = $values['format'];
    }
}, null, 'MakinaCorpus\\Normalizer\\Benchmarks\\MockTextWithFormat');

G402f10984f89322030223b7630346c10ce9bf0a4::$extract0 = \Closure::bind(static function ($object, &$values) {
    $values['text'] = $object->text;
    $values['format'] = $object->format;
}, null, 'MakinaCorpus\\Normalizer\\Benchmarks\\MockTextWithFormat');

I still keep and use this fork for two reasons:

a branch keeps php 5.6 compatibility (real world if not perfect, and clients don't always want or can upgrade their environments),
I don't want to keep all those tricky dependencies (visitor, ast and such).

You can see this code in the master branch of the project: https://github.com/makinacorpus/generated-hydrator/blob/master/src/GeneratedHydrator/ClassGenerator/PHP5HydratorGenerator.php

I intend to move the generated code to PHP7 in order to use ?? or ?: operators, I'm actually investigating this.

pounard · 2019-07-13T15:13:33Z

Needless to say I would be very happy to reconcile my forked project with this one, so I can help if you wish.

kelunik · 2021-01-10T22:36:11Z

I've written a small PoC based on the ideas here today. https://gist.github.com/kelunik/3548c20696726cee2dbd69631b344557

Without code generation but direct closures instead, it's faster than the current code in my first benchmark.

➜ vendor/bin/phpbench run --filter AllPrivate
PhpBench @git_tag@. Running benchmarks.
Using configuration file: /home/kelunik/GitHub/Ocramius/GeneratedHydrator/phpbench.json

\GeneratedHydratorBenchmark\AllPrivateClassHydrationRefBench

    benchConsume............................I199 [μ Mo]/r: 0.447 0.416 (μs) [μSD μRSD]/r: 0.105μs 23.59%

\GeneratedHydratorBenchmark\LaminasHydrationBench

    benchMethodAllPrivate...................I199 [μ Mo]/r: 38.585 35.965 (μs) [μSD μRSD]/r: 7.311μs 18.95%
    benchReflectionAllPrivate...............I199 [μ Mo]/r: 5.680 5.220 (μs) [μSD μRSD]/r: 1.563μs 27.52%

\GeneratedHydratorBenchmark\AllPrivateClassHydrationBench

    benchConsume............................I199 [μ Mo]/r: 0.579 0.510 (μs) [μSD μRSD]/r: 0.220μs 37.92%

4 subjects, 800 iterations, 400 revs, 0 rejects, 0 failures, 0 warnings
(best [mean mode] worst) = 0.400 [11.323 10.528] 1.080 (μs)
⅀T: 9,058.280μs μSD/r 2.300μs μRSD/r: 26.995%

Ocramius added the enhancement label Jan 14, 2017

Ocramius added this to the 2.1.0 milestone Jan 14, 2017

Ocramius self-assigned this Jan 14, 2017

VolCh added a commit to VolCh/GeneratedHydrator that referenced this issue Jun 13, 2018

start implement Ocramius#64

eeb8b55

VolCh linked a pull request Jun 13, 2018 that will close this issue

[wip] start implement #64 #78

Open

Ocramius removed this from the 3.0.0 milestone Mar 4, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generated hydrator and extractor should be a closure, not a class #64

Generated hydrator and extractor should be a closure, not a class #64

Ocramius commented Jan 14, 2017 •

edited

Loading

pounard commented Jan 14, 2017 •

edited

Loading

pounard commented Jul 13, 2019

pounard commented Jul 13, 2019

kelunik commented Jan 10, 2021

Generated hydrator and extractor should be a closure, not a class #64

Generated hydrator and extractor should be a closure, not a class #64

Comments

Ocramius commented Jan 14, 2017 • edited Loading

pounard commented Jan 14, 2017 • edited Loading

Nominal use case, no inheritance

Inheritance with visible properties use case

Inheritance with hidden properties use case

Deep inheritance with hidden properties use case

Deep inheritance with mixed properties use case

General notes

pounard commented Jul 13, 2019

pounard commented Jul 13, 2019

kelunik commented Jan 10, 2021

Ocramius commented Jan 14, 2017 •

edited

Loading

pounard commented Jan 14, 2017 •

edited

Loading