Optimized DispatchingEntityIdParser #764
Conversation
| @@ -27,6 +27,10 @@ class DispatchingEntityIdParser implements EntityIdParser { | |||
| * @param callable[] $idBuilders | |||
| */ | |||
| public function __construct( array $idBuilders ) { | |||
| if ( $this->idBuilders === [] ) { | |||
| throw new InvalidArgumentException( '$idBuilders must not be empty' ); | |||
There was a problem hiding this comment.
Uh. I find this a little problematic. I believe this was done as it was before for a reason: The idea was to allow a setup with no custom entity types. As long as the parse method is not called, it is not a problem to have an empty ID parser.
Even if such a situation can never happen in production, this change makes this entire patch a breaking one. We could avoid this by keeping the previous handling of empty arrays.
The least minimum is to documented this as a @throws.
There was a problem hiding this comment.
Moved back, this probably hardly has any performance impact. It mostly looked weird to me.
| } catch ( InvalidArgumentException $ex ) { | ||
| // Only do this assert in case something is wrong, EntityId::splitSerialization already | ||
| // checks for string. | ||
| $this->assertIdIsString( $idSerialization ); |
There was a problem hiding this comment.
Why keep this additional check inside the catch? This looks awkward. What does this do in addition to what splitSerialization already does? Let's remove it, please.
There was a problem hiding this comment.
changing it will change the Exception message slightly, but I guess that's ok.
This made the function quite a bit faster: 14.48% 48797.778 817218 - Wikibase\DataModel\Entity\DispatchingEntityIdParser::parse to 12.64% 42330.389 817218 - Wikibase\DataModel\Entity\DispatchingEntityIdParser::parse when JSON-dumping 10,000 Wikidata entities. Related to https://phabricator.wikimedia.org/T177486.
mariushoch
force-pushed
the
DispatchingEntityIdParser-perf
branch
from
f0c55d6
to
db79e30
Compare
|
Note: This is no longer breaking |
This made the function quite a bit faster: 14.48% 48797.778 817218 - Wikibase\DataModel\Entity\DispatchingEntityIdParser::parse to 12.64% 42330.389 817218 - Wikibase\DataModel\Entity\DispatchingEntityIdParser::parse when JSON-dumping 10,000 Wikidata entities. Related to https://phabricator.wikimedia.org/T177486.
This made the function quite a bit faster:
to
when JSON-dumping 10,000 Wikidata entities.
Related to https://phabricator.wikimedia.org/T177486.