Skip to content

Commit 2f22d91

Browse files
adrijerowork
authored andcommitted
Index AST nodes by class name for O(1) getNodeByClassName lookups
getNodeByClassName() did two linear scans over every node (one by class name, one by alias) on each call. The schema builder/resolvers call it once per referenced type while resolving fields, so on a non-trivial schema this becomes a hot path: for one real schema (253 nodes) a single request triggered ~3,300 calls, i.e. hundreds of thousands of getClassName() comparisons, dominating the request in a profiler. Build a class-name and an alias index once in the constructor (the Ast is immutable) and resolve lookups from them. The class-name-before-alias resolution order is preserved.
1 parent eac7853 commit 2f22d91

2 files changed

Lines changed: 69 additions & 22 deletions

File tree

src/Ast.php

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,35 @@
2525
*/
2626
private array $nodes;
2727

28+
/**
29+
* Index of nodes by class name, with aliases as a fallback, for O(1) lookups in getNodeByClassName().
30+
*
31+
* @var array<class-string, Node>
32+
*/
33+
private array $nodesByName;
34+
2835
public function __construct(Node ...$nodes)
2936
{
3037
$this->nodes = array_values($nodes);
38+
39+
// Index class names first so they take precedence over aliases.
40+
$nodesByName = [];
41+
42+
foreach ($this->nodes as $node) {
43+
$nodesByName[$node->getClassName()] ??= $node;
44+
}
45+
46+
foreach ($this->nodes as $node) {
47+
if ($node instanceof AliasedNode) {
48+
$alias = $node->getAlias();
49+
50+
if ($alias !== null) {
51+
$nodesByName[$alias] ??= $node;
52+
}
53+
}
54+
}
55+
56+
$this->nodesByName = $nodesByName;
3157
}
3258

3359
/**
@@ -47,28 +73,7 @@ public function getNodesByNodeType(string $nodeType): array
4773
*/
4874
public function getNodeByClassName(string $className): ?Node
4975
{
50-
foreach ($this->nodes as $node) {
51-
if ($node->getClassName() !== $className) {
52-
continue;
53-
}
54-
55-
return $node;
56-
}
57-
58-
// Try to retrieve node by alias
59-
foreach ($this->nodes as $node) {
60-
if (!$node instanceof AliasedNode) {
61-
continue;
62-
}
63-
64-
if ($node->getAlias() !== $className) {
65-
continue;
66-
}
67-
68-
return $node;
69-
}
70-
71-
return null;
76+
return $this->nodesByName[$className] ?? null;
7277
}
7378

7479
/**

tests/AstTest.php

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use Jerowork\GraphqlAttributeSchema\Node\InterfaceTypeNode;
1313
use Jerowork\GraphqlAttributeSchema\Node\MutationNode;
1414
use Jerowork\GraphqlAttributeSchema\Node\QueryNode;
15+
use Jerowork\GraphqlAttributeSchema\Node\ScalarNode;
1516
use Jerowork\GraphqlAttributeSchema\Node\TypeNode;
1617
use Jerowork\GraphqlAttributeSchema\Node\TypeReference\ObjectTypeReference;
1718
use Jerowork\GraphqlAttributeSchema\Node\TypeReference\ScalarTypeReference;
@@ -22,6 +23,7 @@
2223
use Jerowork\GraphqlAttributeSchema\Test\Doubles\InterfaceType\TestOtherInterfaceType;
2324
use Jerowork\GraphqlAttributeSchema\Test\Doubles\Mutation\TestMutation;
2425
use Jerowork\GraphqlAttributeSchema\Test\Doubles\Query\TestQuery;
26+
use Jerowork\GraphqlAttributeSchema\Test\Doubles\Scalar\TestScalarType;
2527
use Jerowork\GraphqlAttributeSchema\Test\Doubles\Type\Loader\TestTypeLoader;
2628
use Jerowork\GraphqlAttributeSchema\Test\Doubles\Type\TestType;
2729
use Override;
@@ -104,6 +106,46 @@ public function itShouldGetNodeByTypeId(): void
104106
{
105107
self::assertSame($this->typeNode, $this->ast->getNodeByClassName(TestType::class));
106108
self::assertSame($this->enumNode2, $this->ast->getNodeByClassName(TestAnotherEnumType::class));
109+
self::assertNull($this->ast->getNodeByClassName(TestQuery::class));
110+
}
111+
112+
#[Test]
113+
public function itShouldGetNodeByAliasWhenThereIsNoClassNameMatch(): void
114+
{
115+
$ast = new Ast(
116+
$scalarNode = new ScalarNode(
117+
TestScalarType::class,
118+
'TestScalar',
119+
null,
120+
TestAnotherEnumType::class,
121+
),
122+
);
123+
124+
self::assertSame($scalarNode, $ast->getNodeByClassName(TestScalarType::class));
125+
self::assertSame($scalarNode, $ast->getNodeByClassName(TestAnotherEnumType::class));
126+
}
127+
128+
#[Test]
129+
public function itShouldPreferAClassNameMatchOverAnAliasMatch(): void
130+
{
131+
$ast = new Ast(
132+
new ScalarNode(
133+
TestScalarType::class,
134+
'TestScalar',
135+
null,
136+
TestType::class,
137+
),
138+
$typeNode = new TypeNode(
139+
TestType::class,
140+
'type',
141+
null,
142+
[],
143+
null,
144+
[],
145+
),
146+
);
147+
148+
self::assertSame($typeNode, $ast->getNodeByClassName(TestType::class));
107149
}
108150

109151
#[Test]

0 commit comments

Comments
 (0)