drupal_valid_test_ua() only assigns a value to its static on a positive match.
This means that the entire function is re-executed under normal operations whenever invoked.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | test_valid_ua_static-1411114-d7-9.patch | 295 bytes | albert volkman |
| drupal8.test-valid-ua-static.0.patch | 315 bytes | sun |
Comments
Comment #1
bleen commentedThis is a no-brainer ... patch applies fine.
As soon as testbot says ok, RTBC
Comment #2
amateescu commentedRTBC it is.
Comment #3
David_Rothstein commentedThis seems harmless, but the only thing it really bypasses in that case is a single call to preg_match()... Is it really worth statically caching that?
Comment #4
dries commentedI don't think it is worth caching, actually.
Comment #5
sunIn case of a positive preg_match(), further processing and calculations are executed. And those should be cached.
This is a mere Boolean static value, which has no weight at all in terms of memory.
Comment #6
David_Rothstein commentedBut a positive preg_match() only occurs for a valid simpletest request (which is already cached) or for someone trying to hack the site by pretending to be simpletest (which is pretty unlikely)... right?
That said, it definitely seems harmless in terms of memory usage.
Comment #8
catchI've gone ahead and committed this, since we're not adding any new static caching here, just fixing the existing one.
I'm not sure it's worth static caching this, so I opened #1436684: Remove static cache in drupal_valid_test_ua(), however there are an increasing number of patches in the queue trying to check this for bizarre simpletest edge cases, so I'd not want to remove it without discussing a bit more.
Comment #9
albert volkman commentedEasy D7 backport :)
Comment #10
sunComment #11
webchickCommitted and pushed to 7.x. Thanks!