Skip to content

Commit

Permalink
Improve performance by doing less cloning
Browse files Browse the repository at this point in the history
Ref #1837
  • Loading branch information
muglug committed Jun 26, 2019
1 parent 70a1696 commit c66a106
Show file tree
Hide file tree
Showing 10 changed files with 54 additions and 8 deletions.
8 changes: 4 additions & 4 deletions src/Psalm/Context.php
Expand Up @@ -301,10 +301,6 @@ public function __construct($self = null)
*/ */
public function __clone() public function __clone()
{ {
foreach ($this->vars_in_scope as &$type) {
$type = clone $type;
}

foreach ($this->clauses as &$clause) { foreach ($this->clauses as &$clause) {
$clause = clone $clause; $clause = clone $clause;
} }
Expand Down Expand Up @@ -353,6 +349,8 @@ public function update(
continue; continue;
} }


$existing_type = clone $existing_type;

// if the type changed within the block of statements, process the replacement // if the type changed within the block of statements, process the replacement
// also never allow ourselves to remove all types from a union // also never allow ourselves to remove all types from a union
if ((!$new_type || !$old_type->equals($new_type)) if ((!$new_type || !$old_type->equals($new_type))
Expand All @@ -366,6 +364,8 @@ public function update(


$updated_vars[$var_id] = true; $updated_vars[$var_id] = true;
} }

$this->vars_in_scope[$var_id] = $existing_type;
} }
} }
} }
Expand Down
3 changes: 3 additions & 0 deletions src/Psalm/Internal/Analyzer/ClassAnalyzer.php
Expand Up @@ -1567,6 +1567,9 @@ private function analyzeClassMethod(
); );


$method_context = clone $class_context; $method_context = clone $class_context;
foreach ($method_context->vars_in_scope as $context_var_id => $context_type) {
$method_context->vars_in_scope[$context_var_id] = clone $context_type;
}
$method_context->collect_exceptions = $config->check_for_throws_docblock; $method_context->collect_exceptions = $config->check_for_throws_docblock;


$method_analyzer->analyze( $method_analyzer->analyze(
Expand Down
Expand Up @@ -177,6 +177,10 @@ public static function analyze(


$foreach_context = clone $context; $foreach_context = clone $context;


foreach ($foreach_context->vars_in_scope as $context_var_id => $context_type) {
$foreach_context->vars_in_scope[$context_var_id] = clone $context_type;
}

$foreach_context->inside_loop = true; $foreach_context->inside_loop = true;
$foreach_context->inside_case = false; $foreach_context->inside_case = false;


Expand Down
10 changes: 10 additions & 0 deletions src/Psalm/Internal/Analyzer/Statements/Block/LoopAnalyzer.php
Expand Up @@ -94,6 +94,11 @@ public static function analyze(


if ($assignment_depth === 0 || $has_break_statement) { if ($assignment_depth === 0 || $has_break_statement) {
$inner_context = clone $loop_scope->loop_context; $inner_context = clone $loop_scope->loop_context;

foreach ($inner_context->vars_in_scope as $context_var_id => $context_type) {
$inner_context->vars_in_scope[$context_var_id] = clone $context_type;
}

$inner_context->loop_scope = $loop_scope; $inner_context->loop_scope = $loop_scope;


$inner_context->parent_context = $loop_scope->loop_context; $inner_context->parent_context = $loop_scope->loop_context;
Expand Down Expand Up @@ -163,6 +168,11 @@ public static function analyze(
$pre_loop_context = clone $loop_scope->loop_context; $pre_loop_context = clone $loop_scope->loop_context;


$inner_context = clone $loop_scope->loop_context; $inner_context = clone $loop_scope->loop_context;

foreach ($inner_context->vars_in_scope as $context_var_id => $context_type) {
$inner_context->vars_in_scope[$context_var_id] = clone $context_type;
}

$inner_context->parent_context = $loop_scope->loop_context; $inner_context->parent_context = $loop_scope->loop_context;
$inner_context->loop_scope = $loop_scope; $inner_context->loop_scope = $loop_scope;


Expand Down
Expand Up @@ -272,7 +272,7 @@ function ($var_id) use ($original_vars_in_scope) {
if ($var_id && isset($pre_op_context->vars_in_scope[$var_id])) { if ($var_id && isset($pre_op_context->vars_in_scope[$var_id])) {
$left_inferred_reconciled = Reconciler::reconcileTypes( $left_inferred_reconciled = Reconciler::reconcileTypes(
'!falsy', '!falsy',
$pre_op_context->vars_in_scope[$var_id], clone $pre_op_context->vars_in_scope[$var_id],
'', '',
$statements_analyzer, $statements_analyzer,
$context->inside_loop, $context->inside_loop,
Expand Down
Expand Up @@ -349,6 +349,8 @@ public static function analyze(
) { ) {
$keys_to_remove = []; $keys_to_remove = [];


$class_type = clone $class_type;

foreach ($class_type->getTypes() as $key => $type) { foreach ($class_type->getTypes() as $key => $type) {
if (!$type instanceof TNamedObject) { if (!$type instanceof TNamedObject) {
$keys_to_remove[] = $key; $keys_to_remove[] = $key;
Expand Down
Expand Up @@ -2678,9 +2678,11 @@ private static function coerceValueAfterGatekeeperArgument(
return; return;
} }


if ($param_type->from_docblock && !$input_type->isMixed()) { if ($param_type->from_docblock && !$input_type->hasMixed()) {
$input_type_changed = false; $input_type_changed = false;


$input_type = clone $input_type;

foreach ($param_type->getTypes() as $param_atomic_type) { foreach ($param_type->getTypes() as $param_atomic_type) {
if ($param_atomic_type instanceof Type\Atomic\TGenericObject) { if ($param_atomic_type instanceof Type\Atomic\TGenericObject) {
foreach ($input_type->getTypes() as $input_atomic_type) { foreach ($input_type->getTypes() as $input_atomic_type) {
Expand All @@ -2690,6 +2692,7 @@ private static function coerceValueAfterGatekeeperArgument(
foreach ($input_atomic_type->type_params as $i => $type_param) { foreach ($input_atomic_type->type_params as $i => $type_param) {
if ($type_param->isEmpty() && isset($param_atomic_type->type_params[$i])) { if ($type_param->isEmpty() && isset($param_atomic_type->type_params[$i])) {
$input_type_changed = true; $input_type_changed = true;

$input_atomic_type->type_params[$i] = clone $param_atomic_type->type_params[$i]; $input_atomic_type->type_params[$i] = clone $param_atomic_type->type_params[$i];
} }
} }
Expand All @@ -2710,6 +2713,8 @@ private static function coerceValueAfterGatekeeperArgument(
); );


if ($var_id) { if ($var_id) {
$input_type = clone $input_type;

if ($input_type->isNullable() && !$param_type->isNullable()) { if ($input_type->isNullable() && !$param_type->isNullable()) {
$input_type->removeType('null'); $input_type->removeType('null');
} }
Expand All @@ -2720,7 +2725,7 @@ private static function coerceValueAfterGatekeeperArgument(
foreach ($input_type->getTypes() as $atomic_type) { foreach ($input_type->getTypes() as $atomic_type) {
$atomic_type->from_docblock = false; $atomic_type->from_docblock = false;
} }
} elseif ($input_type->isMixed() && $signature_param_type) { } elseif ($input_type->hasMixed() && $signature_param_type) {
$input_type = clone $signature_param_type; $input_type = clone $signature_param_type;


if ($input_type->isNullable()) { if ($input_type->isNullable()) {
Expand Down
Expand Up @@ -300,7 +300,7 @@ function (\Psalm\Internal\Clause $c) use ($mixed_var_ids) {
} elseif (isset($stmt->cond->inferredType)) { } elseif (isset($stmt->cond->inferredType)) {
$if_return_type_reconciled = Reconciler::reconcileTypes( $if_return_type_reconciled = Reconciler::reconcileTypes(
'!falsy', '!falsy',
$stmt->cond->inferredType, clone $stmt->cond->inferredType,
'', '',
$statements_analyzer, $statements_analyzer,
$context->inside_loop, $context->inside_loop,
Expand Down
4 changes: 4 additions & 0 deletions src/Psalm/Type.php
Expand Up @@ -1355,6 +1355,10 @@ public static function combineUnionTypes(
bool $allow_mixed_union = true, bool $allow_mixed_union = true,
int $literal_limit = 500 int $literal_limit = 500
) { ) {
if ($type_1 === $type_2) {
return $type_1;
}

if ($type_1->isVanillaMixed() && $type_2->isVanillaMixed()) { if ($type_1->isVanillaMixed() && $type_2->isVanillaMixed()) {
$combined_type = Type::getMixed(); $combined_type = Type::getMixed();
} else { } else {
Expand Down
18 changes: 18 additions & 0 deletions tests/FunctionCallTest.php
Expand Up @@ -1740,6 +1740,24 @@ function getObject() : iterable{
$a = ["1.0", "2.0"]; $a = ["1.0", "2.0"];
uksort($a, "version_compare");' uksort($a, "version_compare");'
], ],
'coerceToObjectAfterBeingCalled' => [
'<?php
class Foo {
public function bar() : void {}
}
function takesFoo(Foo $foo) : void {}
/** @param mixed $f */
function takesMixed($f) : void {
if (rand(0, 1)) {
$f = new Foo();
}
/** @psalm-suppress MixedArgument */
takesFoo($f);
$f->bar();
}'
],
]; ];
} }


Expand Down

0 comments on commit c66a106

Please sign in to comment.