Skip to content

Conversation

@soyuka
Copy link
Contributor

@soyuka soyuka commented Aug 21, 2025

Q A
Branch? 7.4
Bug fix? yes
New feature? no
Deprecations? no
Issues na
License MIT

Profile:

20250821_22h15m27s_grim 20250821_22h15m12s_grim
phpbench ```php namespace Symfony\Component\ObjectMapper\Benchmarks;

use PhpBench\Attributes as Bench;
use Symfony\Component\ObjectMapper\ObjectMapper;
use Symfony\Component\ObjectMapper\Tests\Fixtures\A;
use Symfony\Component\ObjectMapper\Tests\Fixtures\C;
use Symfony\Component\ObjectMapper\Tests\Fixtures\D;

#[Bench\BeforeMethods('setUp')]
class ObjectMapperBench
{
private ObjectMapper $objectMapper;
private A $sourceObject;

public function setUp(): void
{
    $this->objectMapper = new ObjectMapper();

    $d = new D(baz: 'foo', bat: 'bar');
    $c = new C(foo: 'foo', bar: 'bar');
    $this->sourceObject = new A();
    $this->sourceObject->foo = 'test';
    $this->sourceObject->transform = 'test';
    $this->sourceObject->baz = 'me';
    $this->sourceObject->notinb = 'test';
    $this->sourceObject->relation = $c;
    $this->sourceObject->relationNotMapped = $d;
}

#[Bench\Revs(100)]
#[Bench\Iterations(5)]
public function benchObjectMapper(): void
{
    $this->objectMapper->map($this->sourceObject);
}

}

</details>

@OskarStark
Copy link
Contributor

The table in the PR header does not match our standards

@soyuka
Copy link
Contributor Author

soyuka commented Aug 22, 2025

The table in the PR header does not match our standards

I understood that as long as MIT was present this was ok, I removed the columns that are not relevant...

Copy link
Contributor

@Spomky Spomky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the $reflectionClassCache keys are only strings, the SplObjectStorage is not needed right?
(because of $this->reflectionClassCache[$object::class] it will throw an exception)

Copy link
Contributor

@Spomky Spomky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!👍

Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good iterative improvement. To get the best performance, this cache should be generated in the DIC cache and injected at instantiation.

@soyuka
Copy link
Contributor Author

soyuka commented Aug 25, 2025

@GromNaN probably that warming up #61515 would be even better :)

@fabpot
Copy link
Member

fabpot commented Aug 27, 2025

As this is a perf improvement, it cannot be considered as a bug and as such, it will be merged in 7.4.

@fabpot fabpot modified the milestones: 7.3, 7.4 Aug 27, 2025
@fabpot fabpot changed the base branch from 7.3 to 7.4 August 27, 2025 09:12
@fabpot
Copy link
Member

fabpot commented Aug 27, 2025

Thank you @soyuka.

@fabpot fabpot merged commit 07273df into symfony:7.4 Aug 27, 2025
This was referenced Oct 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants