Postpone native guessers instantiation in MimeTypeGuesser. #27323
Conversation
| */ | ||
| private function getGuessers() | ||
| { | ||
| if (!$this->nativeGuessersLoaded) { |
There was a problem hiding this comment.
No need for new variable, just use: if (!$this->guessers) {}
There was a problem hiding this comment.
This is not going to work in case someone registers new guessers later with register(). Then native guessers will never be appended and that might affect the guessing process since there will be no default guessers.
| if (!$this->nativeGuessersLoaded) { | ||
| // register all natively provided mime type guessers. | ||
|
|
||
| if (FileinfoMimeTypeGuesser::isSupported()) { |
There was a problem hiding this comment.
Just asking: couldn't we detect if native guessers are available during the application compilation with a compiler pass?
There was a problem hiding this comment.
Yes, I understand what you are proposing. Yes, this can be done once while compilation. But then it's better not to use a Singleton and have just a normal ChainGuesser, empty by default. The problem is that MimeTypeGuesser as a Singleton is already used everywhere, so it's easier to fix it this way.
|
Status: Needs Review |
|
This is a new feature (or put another way, this is not a bug fix), so it must be on the master branch. |
|
Agree. I will rebase it at the end of the week. |
| // register all natively provided mime type guessers. | ||
|
|
||
| if (FileinfoMimeTypeGuesser::isSupported()) { | ||
| $this->guessers[] = new FileinfoMimeTypeGuesser(); |
There was a problem hiding this comment.
this changes the existing behavior, as they are now registered at the end while the existing logic registers them before custom ones.
There was a problem hiding this comment.
In existing code native guessers are prepended to an empty array in a private constructor. By the time guess is called, native loaders are always at the end of the array.
In this PR they are added later. So to have the same behavior, I add them to the end of the array in a reversed order.
There was a problem hiding this comment.
ah, I forgot the fact that register was prepending the guessers
|
Status: Needs Review |
|
Oops I'm sorry I missed this PR and made #27359 instead. Thank you @vudaltsov for working on this anyway! |
In this PR I made singleton
MimeTypeGuesserlazy. Now it registersFileinfoMimeTypeGuesserandFileBinaryMimeTypeGuesseronly whenguess()is called. Should improve performance.I don't think it's a feature - more a fix... Can this be merged into 2.7?