Optimise performance of getAttribute() calls by caching status of sandbox for the current environment #2359
Conversation
of sandbox for the current environment
|
The issue with tests is probably that the same object hash is reused later by a different Environment object |
|
I'll need to check what I've done later - the object hash was fabpot's suggestion and I ran phpunit before generating PR and had it passing... So I wonder at what point I've got confused! |
| @@ -1463,15 +1463,25 @@ function twig_get_attribute(Twig_Environment $env, Twig_Source $source, $object, | |||
| throw new Twig_Error_Runtime('Accessing Twig_Template attributes is forbidden.'); | |||
| } | |||
|
|
|||
| static $extensionSandbox = array(); | |||
There was a problem hiding this comment.
would it better introduce private non-static class property hasSandboxExtension or sandboxExtension?
There was a problem hiding this comment.
I've put up a different implementation in #2361 . Have also added what would appear to be a missing call to update the options hash. In terms of the non-static class property - the code I was originally modifying wasn't within a class..
|
Regarding the phpunit test failures.... I'm not sure I understand: PHPUnit 4.2.6 by Sebastian Bergmann. Configuration read from twig\Twig\phpunit.xml.dist Testing Twig Test Suite Time: 46.29 seconds, Memory: 24.00MB OK, but incomplete, skipped, or risky tests! |
| @@ -1463,15 +1463,25 @@ function twig_get_attribute(Twig_Environment $env, Twig_Source $source, $object, | |||
| throw new Twig_Error_Runtime('Accessing Twig_Template attributes is forbidden.'); | |||
| } | |||
|
|
|||
| static $extensionSandbox = array(); | |||
| $envHash = spl_object_hash($env); | |||
There was a problem hiding this comment.
in terms of the spl_object_hash - does it actually makes more sense to look at the value of $this->optionsHash on the environment? [that would appear to get updated whenever a new extension is added]
This PR was squashed before being merged into the 2.x branch (closes #2635). Discussion ---------- Optimize the performance of the dot operator This is an alternative to #2359 and #2361, and closes #2326. Basically, we don't need to call `hasExtension()` at runtime for the sandbox check when calling the dot operator as we know if it's enabled at compile time (the cache hash depends on enables extensions). My benchmarks shows a *huge* performance improvement (**around 20%**) by doing so (especially when not using the sandbox, which is almost always the case). Commits ------- c917d35 fixed resetting options hash when overriding all extensions 89876d2 optimized the . operator performance
Fixes #2326