I ran some benchmarks with blackfire:

• Comparing the generated normalizer provided in the header and theObjectNormalizer (instanciated with no args, so there's no cache, it's not completely objective), I obtainhttps://blackfire.io/profiles/compare/055fe23b-e2ea-49bc-91be-80d50ce262cd/graph.
• Comparing the generated normalizer and the GetSetMethodNormalizer, I havehttps://blackfire.io/profiles/compare/b65ebb08-1b17-4bc3-a0da-5de8fdb709f0/graph.

The GetSetMethodNormalizer should be faster than the ObjectNormalizer so we have at less a gain of 30%.

The benchmark of the generated normalizer ishttps://blackfire.io/profiles/c0303264-3e68-444c-8687-3b47a8a2205b/graph, according to itFooNormalizer::normalize() takes 22.2 µs.
The benchmark using the GetSetMethodNormalizer ishttps://blackfire.io/profiles/eb6c3ce8-b44b-4b09-9093-167b3b350638/graph, according to itGetSetMethodNormalizer::normalize() takes 127 µs.

It's better without php-parser, it's not really useful here!

How would an Array of Foo's be normalized here? I assume it'd be a chain of ObjectNormalizer and FooNormalizer?

Iseval better (in term of performance) thanrequire_once in your example?
On a real world example, how would the Normalizers be generated so that they can be easily reused?

I've done some hacking on my end, I'll do some bench! Nice job there!

@soyuka the built-in serializer supports array.

Is eval better (in term of performance) than require_once in your example?

I think require_once is better to benefit from the optimisation and opcache, and eval is not available for some hostings.

On a real world example, how would the Normalizers be generated so that they can be easily reused?

We can decide that later but I would use a compiler pass that would register classes based on a glob pattern (defined in the config).

I think require_once is better to benefit from the optimisation and opcache, and eval is not available for some hostings.

Then there's missing a php open tag<?php in the generated classes.

We can decide that later but I would use a compiler pass that would register classes based on a glob pattern (defined in the config).

Might be long to generate every normalizer, from what I understand there your normalizers are groups-independent, so that's a nice thing. Though, I tried to generate data with related entities and I only get the root entity. Takethis gist for example, maybe I missed something!

Can't we move max depth / circular reference and other features shared with non-dumped normalizers in traits to ease the maintenance?

I made some more benchmarks with big collections where each item has 4 relations to another item.

This is in dev mode, with a forced apcu cache. Times are from the profiler, can't run Blackfire on it because it's taking too long.

• Without the normalizers the script is spending 35720ms in the serializer
• With the cached normalizers, this time is reduced to 5897 ms

Another one is getting an 8000 collection of simple objects:

• Without the normalizers we're spending 3112 ms in the serializer
• With the cached normalizers 236 ms

We can see that this speed things a lot!

Here is a gist with the cache warmer and the compiler pass (made them in a hurry).

Edit:
One of our resource with many relations was spending ~300 ms in the serializer. Now it has been reduced to ~30ms.

Note that we could improve performances by avoiding the$this->normalizer->normalize calls. I'm not sure that we have the required metadata to do so for complex data, but we could avoid some calls for the simple data types (boolean, string etc.).

Also, it'd be nice to have a way to improve the generated normalizers (for example if we want metadata fields, for jsonld it could be@type and@id).

/Edit: For the metadata that's being used to generate the class, WDYT about usingproperty-info instead of theclassMetadataFactory? It'll get you more informations about the dataType which will lead to performances improvements (avoid the$this->normalizer->normalize call for simple data).

Note that we could improve performances by avoiding the $this->normalizer->normalize calls. I'm not sure that we have the required metadata to do so for complex data, but we could avoid some calls for the simple data types (boolean, string etc.).

@soyuka that's planed but the PR is already pretty big and I preferred delaying it and having something simpler (certainly a bit slower) but still working.

Can't we move max depth / circular reference and other features shared with non-dumped normalizers in traits to ease the maintenance?

@dunglas what do you think we can share? The code is generated here so except by usingeval it seems complicated to me to share code.

@soyuka that's planed but the PR is already pretty big and I preferred delaying it and having something simpler (certainly a bit slower) but still working.

Isn'tproperty-info a part of the serializer? I see no harm in already replacing theclassMetadataFactory, or maybe that this class should in fact be usingproperty-info would be easier 😄. Nevermind.

The code is generated here so except by using eval it seems complicated to me to share code.

+1 this is hard because the generated code is hard-coding attributes, which most of the code there relies on. One may think it'd be easier to maintain with a php-parser but I'm actually glad you removed it, it's overkill for such thing.

I think we can do a Trait like this one to remove most of the logic from the actual normalizers:

public function isAllowedAttribute($property, &$context) {    $groups = isset($context[ObjectNormalizer::GROUPS]) && is_array($context[ObjectNormalizer::GROUPS]) ? $context[ObjectNormalizer::GROUPS] : null;    if ((null === $groups) && (!isset($context['attributes']) || isset($context['attributes'][$property]) || (is_array($context['attributes']) && in_array($property, $context['attributes'], true)))) {        return true;    }    return false;}public function getValue($value, $property, $format, &$context) {    if (is_scalar($value)) {        return [$property => $value];    } else {        return [$property => $this->normalizer->normalize($value, $format, $context));    }}

@dunglas is this what you had in mind?https://github.com/GuilhemN/symfony/pull/2</details>

@soyuka indeed

I think we can do a Trait like this one to remove most of the logic from the actual normalizers:

I don't really see what we gain except having one more thing to maintain. I don't think it fits everyone's usecase, it is very generic while when you write a custom normalizer, you want something very specific imo.

I don't have experiene with the Serializer component, so maybe there are strong reasons to merge this feature. But, for an outsider like me, this looks wrong for two reasons:

1. If this is done because theSerializer is slow, then we should focus on making it faster ... and not add a PHP code generator to create some optimized runtime code.
2. If this is done because theSerializer is verbose, then we should focus on making it less verbose ... and not add a PHP code generator to create the verbose code faster.

If this is done because the Serializer is slow, then we should focus on making it faster ... and not add a PHP code generator to create some optimized runtime code.

IMHO this is exactly what's being done here, making the serializer faster (because yes it's slow).
The serializer is really well optimized but the PropertyAccessor will always be slower than calling getters directly. In fact, there is nothing faster than using generated accessors to serialize an object.
What bother's you is that this is generating code?

What are you meaning by verbose?

Normalizers are generated at runtime, the first time the user tries to normalize an object.

Does this mean that one cannot use this feature when using a cache that should be read-only? Would it be possible to generate normalizers on warmup (maybe as an opt-in feature)?

Does this mean that one cannot use this feature when using a cache that should be read-only?

Indeed

Would it be possible to generate normalizers on warmup (maybe as an opt-in feature)?

Is it worth it ? I don't think that's really common and it would be easier to implement on the user's side (to implement it in the core we need an efficient way to filter the classes accepted, it would require at least apatterns and anexclude fields imo).

At least I think we should first wait to have a few feedbacks and if that's really a recurrent request only then implement it.

Is it worth it ?

Exactly. It's feasible to do it in a custom cache warmer and it let you control how to choose the class to be normalized.
I must add that when you change your groups, you need to dump normalizers again.

Exactly. It's feasible to do it in a custom cache warmer and it let you control how to choose the class to be normalized.

Lacking time to do this right now, I'd rather leave it to later as the PR is mergeable as is. Else this will unfortunately delay even more this PR.

I must add that when you change your groups, you need to dump normalizers again.

No need, serialization groups are supported by generated normalizers.

PR rebased on master. Is it a no go merging this without a cache warmer?

Do you have some up-to-date benchmark since you rebased? I'm curious to see how much it may improve the ObjectNormalizer perfs compared to#28926.

Although this looks interesting if we only look at the benchmark figures, it looks hard to maintain and does not fit well in what@dunglas exposed in#19374 (comment).

What about following theRouter path and leverage the PHP7 static array cache (see#28865 and#25909 by@nicolas-grekas).

We could leverage the PHP7 static array cache by decorating thePropertyAccessorInterface which is already used byObjectNormalizer and also thePropertyListExtractorInterface when#28775 will be accepted and merged.

Also, I wrote a RFC to improve the performances of the ObjectNormalizer property access part (see#28926).

As theObjectNormalizer is becoming more SOLID, I think it's better to improve performances by small steps instead of hiding real performance problems behind dumped code which will improve the performance of theSerializer but not theObjectNormalizer. Those small improvements will also benefit to anyone using thePropertyAccess andPropertyInfo components either as standalone components or with theFrameworkBundle.

Is this still relevant? There has been a lot of progress on the topic and we might prefer another approach now? Honestly I don't know, I'm just asking :)

You're right, new proposals emerged lately and things seem to be accelerating on their side.

Maintaining this is indeed complicated compared to other solutions. I don't think we'll reach the same performance but if the difference is neglictible and other aspects are in favor of the other solutions it's ok :)

Let's close this and give other proposals a shot! (this weekend eventually)