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

Drop unnecessary InterfaceToCall references in container #348

Conversation

jlabedo
Copy link
Contributor

@jlabedo jlabedo commented Jul 23, 2024

Description

InterfaceToCall are actually registered in the container, and there is a lot of registrations (InterfaceToCall, InterfaceParameter, Attributes).
This PR avoids injecting unnecessary InterfaceToCall references for service activators.
On a medium sized symfony project, I got 1000 objects dropped from the container after this PR, and got a 10% speed boost on cache:clear.

Type of change

  • Performance

Comment on lines 80 to 83
public function getInterfaceToCall(): InterfaceToCall
{
return $this->interceptedMessageProcessor->getInterfaceToCall();
return InterfaceToCall::create($this->getObjectToInvokeOn(), $this->getMethodName());
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I supposed MethodInvocation was public API, so I kept getInterfaceToCall() in the interface and used this fallback.

Copy link
Member

Choose a reason for hiding this comment

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

This will now need to resolve the Interface using reflection on run time. Let's drop this method then, so we won't accidentally use it.

Copy link
Member

Choose a reason for hiding this comment

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

Feel free to merge and do it as follow up

@jlabedo jlabedo changed the title Drop unnecessary interface to call references Drop unnecessary interface to call references in container Jul 23, 2024
@jlabedo jlabedo changed the title Drop unnecessary interface to call references in container Drop unnecessary InterfaceToCall references in container Jul 23, 2024
@jlabedo jlabedo force-pushed the drop-unnecessary-interface-to-call-references branch from 43260fc to b92564a Compare July 24, 2024 07:03
@jlabedo jlabedo requested a review from dgafka July 24, 2024 10:32
@dgafka
Copy link
Member

dgafka commented Jul 26, 2024

I can see in benchmark having better performance on run time with symfony, which dumps the container. I wonder what's the reasoning behind it.
Like initializating new objects is the case here?

@jlabedo
Copy link
Contributor Author

jlabedo commented Jul 26, 2024

I can see in benchmark having better performance on run time with symfony, which dumps the container. I wonder what's the reasoning behind it. Like initializating new objects is the case here?

Maybe, this is the only rationale I have. But yes, InterfaceToCall may be quite expensive to load as there are a lot of objects to get from the container (InterfaceParameters and attributes)

@jlabedo jlabedo merged commit 7c42929 into ecotoneframework:main Jul 26, 2024
30 checks passed
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.

2 participants