Thanks Tobias for this proposal! It's nice to see some performance improvements for this component.

I did just a quick check of the code and I wonder if the code generator building blocks can be a separate component or considerhttps://github.com/nette/php-generator. It looks decoupled from this feature.

Another alternative for PHP code generationnikic/php-parserhttps://github.com/nikic/PHP-Parser/blob/HEAD/doc/component/AST_builders.markdown we already use this library in the Translation component.

That are some good points.
Im not in love with Nette because of its ugly generated PHP code, awkward API, it is opinionated and lack of extensibility. This is of course 100% my opinion and it is from a few years back.

Nikic is way too complex and verbose.

I also looked into re-use the code generator from Config component. But that one is also very opinionated and not generic. I've also heard@mtarld is using a code generator for upcoming PR... I really think a group of contributors should look over Symfony's use of code generators, but I dont think that is a blocker for this PR. (The CodeGenerator here is marked as internal)

Just a quick glance over the description and example, and I noticed this in the generated code:

if (array_key_exists('name',$data)) {$output->name =$data['name'];        }

To my understandingisset() checks have always been (way) faster thanarray_key_exists(). Obviously there is the difference innull handling, but over the years I've seen quite a lot of code where this is optimized intoisset(...) || array_key_exists(...) where normally the (much) quickerisset() check is enough. Might this be an optimization which could be applied here as well?

Great idea though, and looking forward for this 😄

That are some good points. Im not in love with Nette because of its ugly generated PHP code, awkward API, it is opinionated and lack of extensibility. This is of course 100% my opinion and it is from a few years back.

Nikic is way too complex and verbose.

I also looked into re-use the code generator from Config component. But that one is also very opinionated and not generic. I've also heard@mtarld is using a code generator for upcoming PR... I really think a group of contributors should look over Symfony's use of code generators, but I dont think that is a blocker for this PR. (The CodeGenerator here is marked as internal)

I finally usednikic/php-parser in#51718, and I think that's the way to go.
IMHO we can rely on such a stable library. While it is indeed verbose, it'll lighten the codebase a lot, allowing to get rid of builders.
Plus, having an AST instead of a string feels more secure to me, as it avoids typos and can be compatible to any PHP version.

To my understanding isset() checks have always been (way) faster than array_key_exists(). Obviously there is the difference in null handling, but over the years I've seen quite a lot of code where this is optimized into isset(...) || array_key_exists(...) where normally the (much) quicker isset() check is enough. Might this be an optimization which could be applied here as well?

Thats only correct for older PHP versions. In recent versions the opposite is true.

Thats only correct for older PHP versions. In recent versions the opposite is true.

You'll learn something new every day 😄. Ran a quick PHPBench (for loop with 10M iterations doingisset(...) || array_key_exists(...) vsarray_key_exists(...) and split over "key exists" and "key doesn't exist", so 4 benches in total). Andarray_key_exists with a non existing key is even quickest of all. Then comesisset || array_key_exists with an existing key quickly followed byarray_key_exists with an existing key, whileisset || array_key_exists for a non existing key is over 50% slower compared to all of the others.

Usingnikic/php-parser is the way to go IMHO, which is what the maker bundle uses IIRC.

Hey, we spoke a bit together during SymfonyCon hackday. I am working on JanePHP components & AutoMapper. We do a lot of code generation and I love seeing such feature coming to Symfony Serializer ! I didn't saw your PR before some tweet I saw today that's why I didn't understand well all you were talking with Mathias.
I'll also recommend usingnikic/php-parser since it allows you to validate part of the input and will make this code safer ! Also you're not obligated to work only with AST, the library comes with a builder that makes some tasks easier, you can see some examples there:https://github.com/Korbeil/jane-v8/blob/main/src/Component/JsonSchemaGenerator/Generator/ModelGenerator.php#L56-L66 (this is part of the new JanePHP version that I'm working on 😉).

In your implementation I would like to see some sort of hash corresponding to the model your generated the normalizer from, the idea is to know that you generated the normalizer with a specific version of that model and if the model changes, you will generate a new version of it. You can find this logic here if you want an example:https://github.com/jolicode/automapper/blob/9c43e0d3710f202097d30656667041c3cbf8382c/src/MapperMetadata.php#L138-L155.

I'm not sure I am a fan of having the model that holds information about what is going to be generated having also the role to generate stuff, but it fits well in your pull request. Also, do you plan on replacing stuff like ArrayDenormalizer with this PR or a later PR ? I would love being able to have all the normalizer code being generated and not only some part.

this has crazy performance impacts.

me and some collegues wrote something like this a while ago for JMS serializer. if you need more inspiration:https://github.com/liip/serializer

we generate code to convert between object and array and back. we did not use any code generation tool buttwig :-D (but using a code generator is probably the better way to do this). for our complex models, we generate 50k or more lines of PHP that is all dead simple, and thanks to opcache it is quite ok to have such large code files.

depending on event system during the on-the-fly conversion, our generated code will not do the same. it should be possible to trigger the events from the generated code, but we did not use events so we did not make the effort.

Really nice, would love to see that in symfony, i think it's simple enough for a first implementation and other features can be added in subsequent PR.

More optimizations can be done also to increase performance (like directly inject the child normalizer when needed, to have done a similar thing having this direct injection normally provide between 10 to 20% performance in some cases, like a property that is an array of objects) but all of this can be added further.

Also ATM there is now 3 different pull request with a similar goal with the Mapper component and the JsonEncoder component, i don't know which way the symfony team want to go but IMO there is a path were all of this PR can use the same implementation and code base which will drasticly reduce code maintenance and burden to the core team by doing something like :

• create an internal implementation to generate a mapper that read data from an array / object and write it to an array / object, if the implementation use nikic/php-parser it's really easy to change the way the property is read / write on the fly during generation

With this implementation you will be able to achieve the same thing as those 3 PR :

• Normalization Generation -> object to array
• Denormalization Generation -> array to object
• Mapper Generation -> object to object

For the JsonEncode PR this may require some more works to be able to use only specific part of the implementation but i think this is totally doable

Having this will also allow each of this component to share new features (supporting groups, excluding properties, renaming properties, circular loop, etc ...)

* create an internal implementation to generate a mapper that read data from an array / object and write it to an array / object, if the implementation use nikic/php-parser it's really easy to change the way the property is read / write on the fly during generation

this is also the approach we took for our liip serializer-generator:https://github.com/liip/metadata-parser

because we wanted to build a drop-in replacement for JMS, we specifically support JMS annotations, but such a metadata parser could be really generic and provide metadata from the code or any kind of attributes/annotations/metadata mapping files in yaml/xml like doctrine does etc.

Tests are green.
The Psalm error is a false negative.

Any chances to merge into 7.2?

Any chances to merge into 7.2?

I do think it will overlap with coming ObjectMapper (see#51741). First implementation will use Reflection but it's planned (see#54476) to make a second implementation in order to generated mappers that will act as normalizers.

Hi@Nyholm, I've tried to developmy own serializer component based on generated code, exactly the same way you're proposing to Symfony Serializer component. In that moment (5 years back), I made a lot of performance tests with the generated code and without any surprise, the benchmark showed that this is the way to go. If I can contribute with your PR, I'd say to take a look on thecode generator class to see how I generated the normalizer code. Each decision on the generated code was implemented with the light of Blackfire to minimize any bottleneck. You'll notice that to increase even more the performance, some Symfony Serializer logic should be refactored to allow them to be executed by the generated code, like ignored attributes, normalized name, class inheritance and etc. All of this feature has an overhead when used through reflection at runtime and can be improved on the generated code.

This tool helped me a lot when I wrote the component.

How would this interact with#51718?

How would this interact with#51718?

#51718 is for specific (but very common) scenarios. Ie DTOs to/form JSON (and maybe XML). It also skips the normalizer step.

This PR is for the generic use case, to/form all kind of data and does not change the architecture of the serialization process. However, it does not give as impressive results (but almost). Me and@mtarld believes they can/should co-exists.

I made a quick schema to summarize how each PR interact with the Serializer concepts:

ping#51741
ping#51718

ObjectMapper was merged recently and@soyuka tell me that it would be nice to have generated code to improve performance of such mapper.

I do believe it's possible to have the same implementation that does :

• Normalizer
• Denormalizer
• ObjectMapper
• (Maybe JsonEncoder ?)

As the only difference in this is the way to write / read data

The main problem is : where to put this implementation, as other components will have to require this implementation and this is something that should be fully internal (no public extensions points, they are provided by other components as their are specific to the wanted behavior)

I'm not sure what your status are on this actually@Nyholm ?

The main problem is : where to put this implementation, as other components will have to require this implementation and this is something that should be fully internal (no public extensions points, they are provided by other components as their are specific to the wanted behavior)

I see a few ways:

• create a new "internal" component that's used to generate code (could re-use some of the JsonStream component for example)
• create a public component (just like[AstGenerator] [WIP] New Component, add normalizer generator #17516)
• each component does its own thing and some code will be duplicated (actually we need to see if there's really something reusable across components)

Anyways this probably needs an RFC issue to prevent polluting this topic