PHP7 expectations (zero-cost assertions)

List overview All Threads
Download

newer

older

TechCom Radar, 2018-03-14

[x-post] Wikimedia Language team...

David Causse

15 Mar 2018 15 Mar '18

1:42 p.m.

Hi, Sometimes I find adding assert() calls in my code very handy for various reasons: - failures in development mode on some complex code where exposing all the details to unit tests is sometimes hard and/or pointless - readability of the code 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] ). Are there some discussions about this? Is assert() a good practice for the MW code base? If yes would it make sense to benefit from zero-cost assertions in WMF appservers? Thanks! [1] http://php.net/manual/en/function.assert.php#function.assert.expectations [2] https://codesearch.wmflabs.org/search/?q=%5Ctassert%5C(&i=nope&file…

Show replies by date

David Causse

15 Mar 15 Mar

2:17 p.m.

Replying to myself: I just found some discussions here: https://lists.gt.net/wiki/wikitech/378676 I bet that the new assert features in PHP7 don't change the conclusions here, so please ignore my e-mail and sorry for the noise. On Thu, Mar 15, 2018 at 2:42 PM, David Causse <dcausse(a)wikimedia.org> wrote:

...

Hi, Sometimes I find adding assert() calls in my code very handy for various reasons: - failures in development mode on some complex code where exposing all the details to unit tests is sometimes hard and/or pointless - readability of the code 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] ). Are there some discussions about this? Is assert() a good practice for the MW code base? If yes would it make sense to benefit from zero-cost assertions in WMF appservers? Thanks! [1] http://php.net/manual/en/function.assert.php#function. assert.expectations [2] https://codesearch.wmflabs.org/search/?q=%5Ctassert%5C(&i=nope&file… php%24&repos=

Cormac Parle

2:30 p.m.

Was the conclusion “don’t use assert()”? It’s not really that clear to me (fwiw I've always felt a bit squiffy about assert()s in production code, because it’s easy to make a php config mistake and get errors happening all over the place)

...

On 15 Mar 2018, at 14:17, David Causse <dcausse(a)wikimedia.org> wrote: Replying to myself: I just found some discussions here: https://lists.gt.net/wiki/wikitech/378676 I bet that the new assert features in PHP7 don't change the conclusions here, so please ignore my e-mail and sorry for the noise. On Thu, Mar 15, 2018 at 2:42 PM, David Causse <dcausse(a)wikimedia.org> wrote:

Hi, Sometimes I find adding assert() calls in my code very handy for various reasons: - failures in development mode on some complex code where exposing all the details to unit tests is sometimes hard and/or pointless - readability of the code 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] ). Are there some discussions about this? Is assert() a good practice for the MW code base? If yes would it make sense to benefit from zero-cost assertions in WMF appservers? Thanks! [1] http://php.net/manual/en/function.assert.php#function. assert.expectations [2] https://codesearch.wmflabs.org/search/?q=%5Ctassert%5C(&i=nope&file… php%24&repos=

_______________________________________________ Wikitech-l mailing list Wikitech-l(a)lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l

David Causse

2:39 p.m.

The biggest take-away (for me) of the discussion is: Pros: - perf: zero-cost assertions Cons: - the benefits of zero-cost assertion is not worth the risk in a moving code-base like MW. The argument is that even in the case of assert being used properly (to expose strong expectations that cannot be unmet). Problems are too frequently found in production mode so benefiting from zero-cost assertions would be a risk in the sense that we would not be able to detect these errors. On Thu, Mar 15, 2018 at 3:30 PM, Cormac Parle <cparle(a)wikimedia.org> wrote:

...

wrote:

> Hi, > > Sometimes I find adding assert() calls in my code very handy for various > reasons: > - failures in development mode on some complex code where exposing all

the

> details to unit tests is sometimes hard and/or pointless > - readability of the code > > 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

> files [2] ). > > Are there some discussions about this? > Is assert() a good practice for the MW code base? > If yes would it make sense to benefit from zero-cost assertions in WMF > appservers? > > Thanks! > > [1] http://php.net/manual/en/function.assert.php#function. > assert.expectations > [2] https://codesearch.wmflabs.org/search/?q=%5Ctassert%5C(&

i=nope&files=

php%24&repos=

_______________________________________________ Wikitech-l mailing list Wikitech-l(a)lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Brad Jorsch (Anomie)

2:57 p.m.

On Thu, Mar 15, 2018 at 10:39 AM, David Causse <dcausse(a)wikimedia.org> wrote:

...

The biggest take-away (for me) of the discussion is: Pros: - perf: zero-cost assertions Cons: - the benefits of zero-cost assertion is not worth the risk in a moving code-base like MW.

The biggest take-away for me from the discussions is that PHP5 assert()'s reliance on eval()-like operation is bad in a number of ways, and the weird behavior of issuing a warning and then exiting is weird. PHP7's expectations seem like they started fixing those issues, although eval()-like use is still an option and exception-throwing seems to not be the default. -- Brad Jorsch (Anomie) Senior Software Engineer Wikimedia Foundation

David Causse

3:29 p.m.

On Thu, Mar 15, 2018 at 3:57 PM, Brad Jorsch (Anomie) <bjorsch(a)wikimedia.org

...

wrote: PHP7's expectations seem like they started fixing those issues, although eval()-like use is still an option and exception-throwing seems to not be the default.

indeed I must admit that it's rather concerning that assert( 'code' ) will try to evaluate the string arg and I completely overlooked this. I was focused on the perf arguments. But what I understood from discussion is that even if PHP resolves these issues in the future it's unlikely that we will disable assertions in production mode for WMF appservers. So assert() will never be an option for performance hotspots. Thanks for your feedback!

Stas Malyshev

7:38 p.m.

Hi!

...

PHP7's expectations seem like they started fixing those issues, although eval()-like use is still an option and exception-throwing seems to not be the default.

eval mode is deprecated in 7.2 which means that nobody should use it anymore. It's likely would not be deleted until the mythical 8.0 aka "version where the things can be broken" but for all practical purposes it should not be used by anybody now and is as dead as anything can be in 7.x. -- Stas Malyshev smalyshev(a)wikimedia.org

Brad Jorsch (Anomie)

2:35 p.m.

On Thu, Mar 15, 2018 at 9:42 AM, David Causse <dcausse(a)wikimedia.org> wrote:

...

Looking at the MW codebase we don't seem to use assert frequently (only 26 files [2] ).

We generally use the wikimedia/assert library[3] instead. That's used a lot more often.[4] The README.md for that library includes some reasoning for using it over PHP's assert(), with links to past discussion. [3]: https://packagist.org/packages/wikimedia/assert [4]: https://codesearch.wmflabs.org/search/?q=%5CtAssert%3A%3A&i=nope&fi… -- Brad Jorsch (Anomie) Senior Software Engineer Wikimedia Foundation

Stas Malyshev

7:38 p.m.

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

2234

days inactive

2234

days old

wikitech-l@lists.wikimedia.org

Manage subscription

8 comments

4 participants

tags (0)

participants (4)

Brad Jorsch (Anomie)
Cormac Parle
David Causse
Stas Malyshev