Skip to content

Commit

Permalink
Optimize creation from constructor (#186)
Browse files Browse the repository at this point in the history 
When object gets created from constructor, there are always checks for
constructor arguments in context. This eats some of performance,
especially if you don't use context. When looking at the generated code,
I noticed that it can be optimized a little.
I didn't benchmark it a lot, but I did see around 10% more performance
with this optimization

## Comparisons
### Argument from source
Before:
```php
if (\AutoMapper\MapperContext::hasConstructorArgument($context, 'Dto', 'propertyName')) {
    $constructarg = $source->propertyName ?? \AutoMapper\MapperContext::getConstructorArgument($context, 'Dto', 'propertyName');
} else {
    $constructarg = $source->propertyName ?? throw new \AutoMapper\Exception\MissingConstructorArgumentsException('Cannot create an instance ...');
}
```
After:
```php
$constructarg = $source->propertyName ?? (
    \AutoMapper\MapperContext::hasConstructorArgument($context, 'Dto', 'propertyName')
        ? \AutoMapper\MapperContext::getConstructorArgument($context, 'Dto', 'propertyName')
        : throw new \AutoMapper\Exception\MissingConstructorArgumentsException('Cannot create an instance ...')
);
```

### Argument from source with array transformer
Before:
```php
if (\AutoMapper\MapperContext::hasConstructorArgument($context, 'Dto', 'propertyName')) {
    $values = [];
    foreach ($value['propertyName'] ?? [] as $key => $value) {
        $values[$key] = $value;
    }
    $constructarg = count($values) > 0 ? $values : \AutoMapper\MapperContext::getConstructorArgument($context, 'Dto', 'propertyName');
} else {
    $values = [];
    foreach ($value['propertyName'] ?? [] as $key => $value) {
        $values[$key] = $value;
    }
    $constructarg = count($values) > 0 ? $values : throw new \AutoMapper\Exception\MissingConstructorArgumentsException('Cannot create an instance');
}
```
After:
```php
$values = [];
foreach ($value['propertyName'] ?? [] as $key => $value) {
    $values[$key] = $value;
}
$constructarg = count($values) > 0
    ? $values
    : (\AutoMapper\MapperContext::hasConstructorArgument($context, 'Dto', 'propertyName')
        ? \AutoMapper\MapperContext::getConstructorArgument($context, 'Dto', 'propertyName')
        : throw new \AutoMapper\Exception\MissingConstructorArgumentsException('Cannot create an instance')
    );
```
### Argument without source
Before and After here logically the same, I just changed it so code
would be consistent with arguments from source.

Before:
```php
if (\AutoMapper\MapperContext::hasConstructorArgument($context, 'Dto', 'propertyName')) {
    $constructarg = \AutoMapper\MapperContext::getConstructorArgument($context, 'Dto', 'propertyName');
} else {
    throw new \AutoMapper\Exception\MissingConstructorArgumentsException('Cannot create an instance');
}
```
After:
```php
$constructarg = \AutoMapper\MapperContext::hasConstructorArgument($context, 'Dto', 'propertyName')
    ? \AutoMapper\MapperContext::getConstructorArgument($context, 'Dto', 'propertyName')
    : throw new \AutoMapper\Exception\MissingConstructorArgumentsException('Cannot create an instance');
```
  • Loading branch information
@Korbeil
Korbeil authored Oct 5, 2024
2 parents 9d7523d + 94377bf commit 33fa4aa
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 60 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- [GH#187](https://github.com/jolicode/automapper/pull/187) Fix regression after [GH#184](https://github.com/jolicode/automapper/pull/184)
- [GH#192](https://github.com/jolicode/automapper/pull/192) Fix source and context not passed to callable transformer

### Changed
- [GH#186](https://github.com/jolicode/automapper/pull/186) Optimize creation from constructor

## [9.1.2] - 2024-09-03
### Fixed
- [GH#174](https://github.com/jolicode/automapper/pull/174) Fix race condition when writing generated mappers
Expand Down
116 changes: 56 additions & 60 deletions src/Generator/CreateTargetStatementsGenerator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,19 +126,19 @@ private function constructorArguments(GeneratorMetadata $metadata): array
// Find property for parameter
$propertyMetadata = $metadata->getTargetPropertyWithConstructor($constructorParameter->getName());

$createObjectStatement = null;
$propertyStatements = null;
$constructArgument = null;
$constructorName = null;

if (null !== $propertyMetadata) {
[$createObjectStatement, $constructArgument, $constructorName] = $this->constructorArgument($metadata, $propertyMetadata, $constructorParameter);
[$propertyStatements, $constructArgument, $constructorName] = $this->constructorArgument($metadata, $propertyMetadata, $constructorParameter);
}

if (null === $createObjectStatement || null === $constructArgument || null === $constructorName) {
[$createObjectStatement, $constructArgument, $constructorName] = $this->constructorArgumentWithoutSource($metadata, $constructorParameter);
if (null === $propertyStatements || null === $constructArgument || null === $constructorName) {
[$propertyStatements, $constructArgument, $constructorName] = $this->constructorArgumentWithoutSource($metadata, $constructorParameter);
}

$createObjectStatements[] = $createObjectStatement;
$createObjectStatements = [...$createObjectStatements, ...$propertyStatements];
$constructArguments[$constructorName] = $constructArgument;
}

Expand All @@ -158,17 +158,18 @@ private function constructorArguments(GeneratorMetadata $metadata): array
}

/**
* Check if there is a constructor argument in the context, otherwise we use the transformed value.
* If source missing a constructor argument, check if there is a constructor argument in the context, otherwise we use the default value or throw exception.
*
* ```php
* if (MapperContext::hasConstructorArgument($context, $target, 'propertyName')) {
* $constructArg1 = $source->propertyName ?? MapperContext::getConstructorArgument($context, $target, 'propertyName');
* } else {
* $constructArg1 = $source->propertyName;
* }
* {transformation of value}
* $constructarg = $value ?? (
* MapperContext::hasConstructorArgument($context, $target, 'propertyName')
* ? MapperContext::getConstructorArgument($context, $target, 'propertyName')
* : {defaultValueExpr} // default value or throw exception
* )
* ```
*
* @return array{Stmt, Arg, string}|array{null, null, null}
* @return array{Stmt[], Arg, string}|array{null, null, null}
*/
private function constructorArgument(GeneratorMetadata $metadata, PropertyMetadata $propertyMetadata, \ReflectionParameter $parameter): array
{
Expand Down Expand Up @@ -218,45 +219,39 @@ private function constructorArgument(GeneratorMetadata $metadata, PropertyMetada
}

return [
new Stmt\If_(new Expr\StaticCall(new Name\FullyQualified(MapperContext::class), 'hasConstructorArgument', [
new Arg($variableRegistry->getContext()),
new Arg(new Scalar\String_($metadata->mapperMetadata->target)),
new Arg(new Scalar\String_($propertyMetadata->target->property)),
]), [
'stmts' => [
...$propStatements,
new Stmt\Expression($argumentAssignClosure(new Expr\StaticCall(new Name\FullyQualified(MapperContext::class), 'getConstructorArgument', [
new Arg($variableRegistry->getContext()),
new Arg(new Scalar\String_($metadata->mapperMetadata->target)),
new Arg(new Scalar\String_($propertyMetadata->target->property)),
]))),
],
'else' => new Stmt\Else_([
...$propStatements,
new Stmt\Expression($argumentAssignClosure($defaultValueExpr)),
]),
]),
[
...$propStatements,
new Stmt\Expression($argumentAssignClosure(
new Expr\Ternary(
new Expr\StaticCall(new Name\FullyQualified(MapperContext::class), 'hasConstructorArgument', [
new Arg($variableRegistry->getContext()),
new Arg(new Scalar\String_($metadata->mapperMetadata->target)),
new Arg(new Scalar\String_($propertyMetadata->target->property)),
]),
new Expr\StaticCall(new Name\FullyQualified(MapperContext::class), 'getConstructorArgument', [
new Arg($variableRegistry->getContext()),
new Arg(new Scalar\String_($metadata->mapperMetadata->target)),
new Arg(new Scalar\String_($propertyMetadata->target->property)),
]),
$defaultValueExpr,
),
)),
],
new Arg($constructVar, name: new Identifier($parameter->getName())),
$parameter->getName(),
];
}

/**
* Check if there is a constructor argument in the context, otherwise we use the default value.
* Check if there is a constructor argument in the context, otherwise we use the default value or throw exception.
*
* ```
* if (MapperContext::hasConstructorArgument($context, $target, 'propertyName')) {
* $constructArg2 = MapperContext::getConstructorArgument($context, $target, 'propertyName');
* } else {
* $constructArg2 = 'default value';
* // or set to null if the parameter is nullable
* $constructArg2 = null;
* // throw an exception otherwise
* throw new MissingConstructorArgumentsException('Cannot create an instance of "Foo" from mapping data because its constructor requires the following parameters to be present : "$propertyName".', 0, null, ['propertyName'], 'Foo');
* }
* ```php
* $constructarg = MapperContext::hasConstructorArgument($context, $target, 'propertyName')
* ? MapperContext::getConstructorArgument($context, $target, 'propertyName')
* : {defaultValueExpr} // default value or throw exception
* ```
*
* @return array{Stmt, Arg, string}
* @return array{Stmt[], Arg, string}
*/
private function constructorArgumentWithoutSource(GeneratorMetadata $metadata, \ReflectionParameter $constructorParameter): array
{
Expand All @@ -274,28 +269,29 @@ private function constructorArgumentWithoutSource(GeneratorMetadata $metadata, \
]));

if ($constructorParameter->isDefaultValueAvailable()) {
$defaultValueExpr = new Expr\Assign($constructVar, $this->getValueAsExpr($constructorParameter->getDefaultValue()));
$defaultValueExpr = $this->getValueAsExpr($constructorParameter->getDefaultValue());
} elseif ($constructorParameter->allowsNull()) {
$defaultValueExpr = new Expr\Assign($constructVar, new Expr\ConstFetch(new Name('null')));
$defaultValueExpr = new Expr\ConstFetch(new Name('null'));
}

return [
new Stmt\If_(new Expr\StaticCall(new Name\FullyQualified(MapperContext::class), 'hasConstructorArgument', [
new Arg($variableRegistry->getContext()),
new Arg(new Scalar\String_($metadata->mapperMetadata->target)),
new Arg(new Scalar\String_($constructorParameter->getName())),
]), [
'stmts' => [
new Stmt\Expression(new Expr\Assign($constructVar, new Expr\StaticCall(new Name\FullyQualified(MapperContext::class), 'getConstructorArgument', [
new Arg($variableRegistry->getContext()),
new Arg(new Scalar\String_($metadata->mapperMetadata->target)),
new Arg(new Scalar\String_($constructorParameter->getName())),
]))),
],
'else' => new Stmt\Else_([
new Stmt\Expression($defaultValueExpr),
]),
]),
[
new Stmt\Expression(new Expr\Assign($constructVar,
new Expr\Ternary(
new Expr\StaticCall(new Name\FullyQualified(MapperContext::class), 'hasConstructorArgument', [
new Arg($variableRegistry->getContext()),
new Arg(new Scalar\String_($metadata->mapperMetadata->target)),
new Arg(new Scalar\String_($constructorParameter->getName())),
]),
new Expr\StaticCall(new Name\FullyQualified(MapperContext::class), 'getConstructorArgument', [
new Arg($variableRegistry->getContext()),
new Arg(new Scalar\String_($metadata->mapperMetadata->target)),
new Arg(new Scalar\String_($constructorParameter->getName())),
]),
$defaultValueExpr,
))
),
],
new Arg($constructVar, name: new Identifier($constructorParameter->getName())),
$constructorParameter->getName(),
];
Expand Down

0 comments on commit 33fa4aa

Please sign in to comment.