Hi!
But I worry about the perf implications of these lines
of code. I don't
want these assertions to be used to track errors in production mode.
PHP7 introduced expectations which permit to have zero-cost assert() [1]
Looking at the MW codebase we don't seem to use assert frequently (only 26
files [2] ).
According to
https://phabricator.wikimedia.org/T172165 MW 1.31 will
require PHP 7 or HHVM. However, I am not sure how the situation with
zero-cost asserts are in HHVM and therefore in WMF production. Since WMF
production is running HHVM now (this may change in the future but looks
like that's the status for now) we should not rely on something HHVM may
not support.
I would avoid assertions at all in performance-critical code paths. I.e.
something that every microsecond counts. Example: code paths that were
generating RDF dumps had to be hand-tuned to the point we rewrote RDF
generation library, and there's still room for improvement there - and
that's because each code path is hit 40M times when the dump for 40M
entities is generated. However, most of the code in MediaWiki is not
that intensely used and would be just fine with extra function call here
and there.
We can also note that using Wikimedia\Assert precludes using zero cost
asserts in the future, even if we migrate to PHP 7, and the assertion
done this way can not be disabled or turned into zero-cost assertions.
Depending on what function these assertions serve in the code, this may
be a good thing or bad thing. I think in our case it is a good thing -
most parts of the code are not CPU-critical to the tune that extra fcall
would change things, and we have tons of runtime errors anyway and have
a reasonable system around them to handle when they happen. So using
Wikimedia\Assert is likely the way to go for most of MW code.
--
Stas Malyshev
smalyshev(a)wikimedia.org