This change should be reverted

Failed rebase 😅

I would not make this property static, because injected resolver can be different form an instance to another.

We would even improve the performance more: for a given object, regardless of the property, the result of getTypeForMappedObject will always be the same.
We could use SplObjectStorage to call this method only one time for every object.

So we would have two caches, one to know if the current property we are getting the value is the discriminator field of the class and an other one to get the value of this discriminator mapping.

Does the later worth it? It would probably be used only when the same object is present more than once in the serialized objects graph.

getAttributeValue is called for every attribute of a given class, isn't it?

Got it!

I pushed a v2 0bd95ec and the improvement is less significant: https://blackfire.io/profiles/compare/fd66194c-e3a7-4bc1-bea4-c67d2677148f/graph

IMHO we need two levels of cache, one to known if a class has a discriminator, and one to get the property name (roughly what is done in v2).

This will improve the case we have a lots of the same class in the graph and reducing the number of calls to one per object.

WDYT?

EDIT: v2 isn't working. Tests are broken. Will give a look later.

I fixed my v2, tests are now green: https://blackfire.io/profiles/compare/00c4aa38-97d3-41c3-928c-84c5e909cdde/graph

I don't think this works:

• this creates a memory leak since the cache array is never cleared
• but more importantly, object hashes can be reused so that a cached value can map to another object

Both issues could be fixed by clearing this cache in the appropriate place (when exiting any recursive normalization steps)
On PHP 7.2, we should use spl_object_id() btw.

Both issues could be fixed by clearing this cache in the appropriate place (when exiting any recursive normalization steps)

So in the Serializer class. A bit intrusive. (we'll have to introduce a new interface, and call a clearCache() method if the normalizer implements this interface.

Another solution (cleaner) would be to do the resolving in the method calling getAttributeValue(), and pass the result as a new argument of getAttributeValue().
Because it's a protected method, will have to deprecate not passing this new argument.

What about using spl_object_id (with the polyfill for PHP 7.1) for the 2nd point?

For the first point, is it a viable solution to add some property to the $context to keep track of the nested calls and then clear this the cache when exiting the outer call?

//ObjectNormalizer.php
    public function normalize($object, $format = null, array $context = array())
    {
        try {
            $context['discriminator_cache_depth'] = $context['discriminator_cache_depth'] ?? 0 + 1;

            $result = parent::normalize($object, $format, $context);

            --$context['discriminator_cache_depth'];

            return $result;

        } finally {
            if (0 === $context['discriminator_cache_depth']) {
                $this->discriminatorCache = array();
            }
        }
    }

Profiling is not that bad (https://blackfire.io/profiles/compare/cf712867-392c-45ee-a3ff-221e0ba72a01/graph). It reduce the class discriminator overhead (3.4 vs master) from 53.9 to 18.6%.

An other solution would be to ship this only with 4.2 and implement the ResetInterface.

What do you think?

ResetInterface doesn't work as explained: handles can be reused between calls.
The spl_object_id polyfill adds overhead. Since we're tracking perf here, better not.
I'll review iteration v3 :)

If using the polyfill, this PR would still be faster than the current 4.1 state (I'm trying to profile using php 7.1, but I got weird results because of DateTimeZone::getTransitions: https://blackfire.io/profiles/6c0105f5-bf95-49d3-a49d-25adc16cd6fc/graph)!

@dunglas what about going back to v1 slightly improved to store the discriminator attribute per class and call ClassDiscriminatorResolverInterface::getMappingForMappedObject only when getAttributeValue is called when $attribute === $this->discriminatorCache[\get_class($object)]?

This would avoid using spl_object_id/spl_object_hash and the cache will have a finite size and will be reusable between requests.

This would improve perfs a lot, because, we are only computing the cache value once per class (instead of object) and ClassDiscriminatorResolverInterface::getMappingForMappedObject will be called only once per object which is involved in a class discriminator.

I will give this a try and call it v4 ;)