Skip to content

Avoid serializing "resources" in the normalization context #1307

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

Merged
merged 1 commit into from
Aug 5, 2017

Conversation

soyuka
Copy link
Member

@soyuka soyuka commented Aug 4, 2017

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #1303
License MIT
Doc PR na

Because getCacheKey serializes the context, let's avoid serializing the $context['resources'] that can get pretty big.

Solves the performance issue.

@soyuka soyuka requested a review from meyerbaptiste August 4, 2017 08:34
@@ -60,7 +60,9 @@ public function onKernelView(GetResponseForControllerResultEvent $event)

$context = $this->serializerContextBuilder->createFromRequest($request, true, $attributes);
$resources = [];
$context['resources'] = &$resources;
$context['resources'] = function ($iri) use (&$resources) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have the same output has before ? if yes then it seems ok to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes but it's fucked up that a Closure is faster then a reference in this case. There may be a simple explanation with the internal memory pointer doing less work, I would love to understand :p.

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you try to compare the opcode ?

@soyuka soyuka changed the title Fix performance issue because of memory constraint Avoid serializing "resources" in the normalization context Aug 4, 2017
@soyuka soyuka merged commit 4976011 into api-platform:master Aug 5, 2017
@soyuka soyuka deleted the fix/perf/1303 branch August 5, 2017 09:42
hoangnd25 pushed a commit to hoangnd25/core that referenced this pull request Feb 23, 2018
	Avoid serializing "resources" in the normalization context
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants