Skip to content

MimeTypeGuesser proxy. #27307

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.

Sign up for GitHub

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

Jump to bottom
Closed
damour opened this issue May 18, 2018 · 4 comments
Closed

MimeTypeGuesser proxy. #27307

damour opened this issue May 18, 2018 · 4 comments
Labels
Feature HttpFoundation Performance

Comments

@damour
Copy link
Contributor

damour commented May 18, 2018 •
edited
Loading

Description
Serializer instantiation sometimes takes long time (30ms):

_295

Simplest workaround for now is create proxy for MimeTypeGuesser.

Example

serializer.normalizer.mime_type_guesser_proxy:
  class: Application\CommonBundle\Utils\MimeTypeGuesserProxy

serializer.normalizer.data_uri:
  class: Symfony\Component\Serializer\Normalizer\DataUriNormalizer
  arguments: ['@serializer.normalizer.mime_type_guesser_proxy']
  tags:
    - { name: serializer.normalizer, priority: -920 }

final class MimeTypeGuesserProxy implements MimeTypeGuesserInterface
{
    public function guess($path)
    {
        $mimeTypeGuesser = MimeTypeGuesser::getInstance();

        return $mimeTypeGuesser->guess($path);
    }
}

https://github.com/symfony/symfony/blob/v4.0.9/src/Symfony/Bundle/FrameworkBundle/Resources/config/serializer.xml#L33
https://github.com/symfony/symfony/blob/v4.0.9/src/Symfony/Component/HttpFoundation/File/MimeType/MimeTypeGuesser.php#L83
https://github.com/symfony/symfony/blob/v4.0.9/src/Symfony/Component/HttpFoundation/File/MimeType/FileBinaryMimeTypeGuesser.php#L57
@xabbuh
Copy link
Member

xabbuh commented May 18, 2018

I am not sure I understand your issue. Why should your example behave differently at all? Isn't actually executed code still the same?

@damour
Copy link
Contributor Author

damour commented May 18, 2018

In my workaround guess() (and passthru() internally) will be called only when Serializer serialize()/deserialize() executes. Without proxy it calls every time when serializer instantiated but never used.
For example, api platform DeserializeListener requires Serializer in constructor, but if request don't match exit here:
https://github.com/api-platform/core/blob/master/src/EventListener/DeserializeListener.php#L61

So, passthru() executes for all application requests, not only for api platform.

@xabbuh
Copy link
Member

xabbuh commented May 18, 2018

You are right indeed. I wonder if we shouldn't make the core MimeTypeGuesser class lazy instead.

@Aerendir
Copy link
Contributor

I don’t know the internals, but any improvement in performances of the serislized are welcomed for me! It is used heavily by ApiPlatform and everything that makes it Faster is good!

@vudaltsov vudaltsov mentioned this issue May 21, 2018
Closed
@nicolas-grekas nicolas-grekas mentioned this issue May 24, 2018
Merged
fabpot added a commit that referenced this issue May 25, 2018
@fabpot
bug #27359 [HttpFoundation] Fix perf issue during MimeTypeGuesser int…
ab32125 
…ialization (nicolas-grekas)

This PR was merged into the 2.7 branch.

Discussion
----------

[HttpFoundation] Fix perf issue during MimeTypeGuesser intialization

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #27307
| License       | MIT
| Doc PR        | -

introduced in #26886

![image](https://user-images.githubusercontent.com/243674/40451947-918f5358-5ee0-11e8-9f1a-cf707bf3cefa.png)

Commits
-------

f8e7a18 [HttpFoundation] Fix perf issue during MimeTypeGuesser intialization
@fabpot fabpot closed this as completed May 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature HttpFoundation Performance
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@fabpot @javiereguiluz @damour @xabbuh @Aerendir