Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix GH-13988: Storing DOMElement consume 4 times more memory in PHP 8.1 than in PHP 8.0 #15593

Closed
wants to merge 1 commit into from

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Aug 26, 2024

We avoid creating backing storage by using the feature introduced in f78d5cf.
Also have to remove the default value because we don't have backing storage, otherwise we break engine constraints. This doesn't matter in practice because the default value is read anyway via the getter, so it was actually unused anyway...

@nielsdos nielsdos requested a review from kocsismate as a code owner August 26, 2024 20:05
@nielsdos nielsdos force-pushed the fix-13988 branch 2 times, most recently from f68eed1 to 64ae704 Compare August 26, 2024 20:38
…P 8.1 than in PHP 8.0

We avoid creating backing storage by using the feature introduced in
f78d5cf.
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

Looks sensible to me

@@ -1696,6 +2113,7 @@ public function remove(string ...$tokens): void {}
public function toggle(string $token, ?bool $force = null): bool {}
public function replace(string $token, string $newToken): bool {}
public function supports(string $token): bool {}
/** @virtual */
public string $value;
Copy link
Member

@kocsismate kocsismate Aug 27, 2024

Choose a reason for hiding this comment

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

why aren't the two properties listed before the methods as usual?

Copy link
Member Author

Choose a reason for hiding this comment

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

The DOM spec also defines the class structure, and I just follow the same ordering that the spec uses. They tend to group things based on functionality similarities. Although idk either why value isn't on top here.

@nielsdos nielsdos closed this in 88393cf Aug 27, 2024
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.

Storing DOMElement consume 4 times more memory in PHP 8.1 than in PHP 8.0
3 participants