Skip to content

Commit

Permalink
optimize resource merging (#1214)
Browse files Browse the repository at this point in the history
- do not revalidate/normalize resource attributes on merge, since they have already been validated/normalized
- string concat is about 10x faster than sprintf, so use it in ClassConstantAccessor since it's called many times
  • Loading branch information
brettmc authored Jan 15, 2024
1 parent 3928b13 commit 521940d
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 3 deletions.
15 changes: 15 additions & 0 deletions src/SDK/Common/Attribute/AttributesBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,21 @@ public function build(): AttributesInterface
return new Attributes($this->attributes, $this->droppedAttributesCount);
}

public function merge(AttributesInterface $old, AttributesInterface $updating): AttributesInterface
{
$new = $old->toArray();
$dropped = $old->getDroppedAttributesCount() + $updating->getDroppedAttributesCount();
foreach ($updating->toArray() as $key => $value) {
if (count($new) === $this->attributeCountLimit && !array_key_exists($key, $new)) {
$dropped++;
} else {
$new[$key] = $value;
}
}

return new Attributes($new, $dropped);
}

public function offsetExists($offset): bool
{
return array_key_exists($offset, $this->attributes);
Expand Down
1 change: 1 addition & 0 deletions src/SDK/Common/Attribute/AttributesBuilderInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@
interface AttributesBuilderInterface extends ArrayAccess
{
public function build(): AttributesInterface;
public function merge(AttributesInterface $old, AttributesInterface $updating): AttributesInterface;
}
5 changes: 5 additions & 0 deletions src/SDK/Common/Attribute/FilteredAttributesBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ public function build(): AttributesInterface
return new Attributes($attributes->toArray(), $dropped);
}

public function merge(AttributesInterface $old, AttributesInterface $updating): AttributesInterface
{
return $this->builder->merge($old, $updating);
}

public function offsetExists($offset): bool
{
return $this->builder->offsetExists($offset);
Expand Down
2 changes: 1 addition & 1 deletion src/SDK/Common/Util/ClassConstantAccessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ public static function getValue(string $className, string $constantName)

private static function getFullName(string $className, string $constantName): string
{
return sprintf('%s::%s', $className, $constantName);
return $className . '::' . $constantName;
}
}
4 changes: 2 additions & 2 deletions src/SDK/Resource/ResourceInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ public function serialize(): string
public function merge(ResourceInfo $updating): ResourceInfo
{
$schemaUrl = self::mergeSchemaUrl($this->getSchemaUrl(), $updating->getSchemaUrl());
$attributes = $updating->getAttributes()->toArray() + $this->getAttributes()->toArray();
$attributes = Attributes::factory()->builder()->merge($this->getAttributes(), $updating->getAttributes());

return ResourceInfo::create(Attributes::create($attributes), $schemaUrl);
return ResourceInfo::create($attributes, $schemaUrl);
}

/**
Expand Down
13 changes: 13 additions & 0 deletions tests/Unit/SDK/Common/Attribute/AttributesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -169,4 +169,17 @@ public function test_replace_attribute_does_not_increase_dropped_attributes_coun
$attributesBuilder['foo'] = 'bar';
$this->assertEquals(0, $attributesBuilder->build()->getDroppedAttributesCount());
}

public function test_merge_attributes(): void
{
$one = Attributes::factory(1)->builder(['foo' => 'foo', 'foobar' => 'foobar'])->build();
$two = Attributes::factory(2)->builder(['bar' => 'bar', 'baz' => 'baz'])->build();
$merged = Attributes::factory(2)->builder()->merge($one, $two);

$this->assertEquals([
'foo' => 'foo',
'bar' => 'bar',
], $merged->toArray());
$this->assertSame(2, $merged->getDroppedAttributesCount(), 'foobar and baz dropped');
}
}

0 comments on commit 521940d

Please sign in to comment.