-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
compiled interceptors module #22826
base: 2.5-develop
Are you sure you want to change the base?
compiled interceptors module #22826
Changes from 18 commits
2fb8f2a
e0057b6
53709eb
36f069e
7f499e1
d97dba0
cb0a213
d19f4f9
32e3cca
224dd4e
000d0a5
e57df09
f08394d
8071fb2
4948604
75e65ac
5fc398c
1999072
f30e9cf
82dc39f
41ca3b0
944144a
5227be0
d64c89a
16add49
1058be9
ecd9b54
6285273
5cb7dc9
5b85a71
d22a34e
5e62ebc
9c0aad8
bb6cc62
c334bcf
5665eec
c3019f3
dfd3c28
a7fc720
1657b46
07bfca9
7f268ce
8b3cae6
2bc739d
29a6d8e
dca21f7
219382f
f96d546
528408d
aa62a4d
eb1018b
1ef18eb
53e1e8d
75dea82
b422bb1
b40eabe
94a29c7
7fbcecd
53fa5c4
727cfc9
23bdd02
c263481
c082b69
5b57dbc
de6e95b
7bc9f69
d1b674e
f6dccdd
f6e7ac4
9d1f67c
3fb9422
76864e0
8f80b6e
c9f47ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
namespace Magento\Framework\CompiledInterception\Generator; | ||
|
||
use Magento\Framework\App\ObjectManager; | ||
use Magento\Framework\Interception\PluginList\PluginList; | ||
use Magento\Framework\Interception\ObjectManager\ConfigInterface; | ||
use Magento\Framework\ObjectManager\Config\Reader\Dom; | ||
|
||
class CompiledPluginList extends PluginList | ||
{ | ||
/** | ||
* CompiledPluginList constructor. | ||
* @param $objectManager ObjectManager | ||
* @param $scope | ||
* @param null $reader | ||
* @param null $omConfig | ||
* @param null $cachePath | ||
*/ | ||
public function __construct( | ||
$objectManager, | ||
$scope, | ||
$reader = null, | ||
$omConfig = null, | ||
$cachePath = null | ||
) { | ||
if (!$reader || !$omConfig) { | ||
$reader = $objectManager->get(Dom::class); | ||
$omConfig = $objectManager->get(ConfigInterface::class); | ||
} | ||
parent::__construct( | ||
$reader, | ||
new StaticScope($scope), | ||
new FileCache($cachePath), | ||
new \Magento\Framework\ObjectManager\Relations\Runtime(), | ||
fsw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
$omConfig, | ||
new \Magento\Framework\Interception\Definition\Runtime(), | ||
$objectManager, | ||
new \Magento\Framework\ObjectManager\Definition\Runtime(), | ||
['first' => 'global'], | ||
'compiled_plugins_' . $scope, | ||
new NoSerialize() | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. avoiding inheritance will make this class much nicer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This class actually exists so we can use logic from Alternatively I could just modify |
||
} | ||
|
||
/** | ||
* Retrieve plugin Instance | ||
* | ||
* @param string $type | ||
* @param string $code | ||
* @return mixed | ||
* @SuppressWarnings(PHPMD.UnusedFormalParameter) | ||
*/ | ||
public function getPlugin($type, $code) | ||
{ | ||
return null; | ||
} | ||
|
||
/** | ||
* @param $type | ||
* @param $code | ||
* @return mixed | ||
*/ | ||
public function getPluginType($type, $code) | ||
{ | ||
return $this->_inherited[$type][$code]['instance']; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,128 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
namespace Magento\Framework\CompiledInterception\Generator; | ||
|
||
|
||
use Magento\Framework\Config\CacheInterface; | ||
|
||
class FileCache implements CacheInterface | ||
{ | ||
|
||
private $cachePath; | ||
|
||
/** | ||
* FileCache constructor. | ||
* @param null $cachePath | ||
*/ | ||
public function __construct($cachePath = null) | ||
{ | ||
$this->cachePath = ($cachePath === null ? BP . DIRECTORY_SEPARATOR . 'var' . DIRECTORY_SEPARATOR . 'cache' : $cachePath); | ||
} | ||
|
||
|
||
/** | ||
* @param $identifier | ||
* @return string | ||
*/ | ||
private function getCachePath($identifier) | ||
{ | ||
return $this->cachePath . DIRECTORY_SEPARATOR . str_replace('|', '_', $identifier . '.php'); | ||
} | ||
|
||
/** | ||
* Test if a cache is available for the given id | ||
* | ||
* @param string $identifier Cache id | ||
* @return int|bool Last modified time of cache entry if it is available, false otherwise | ||
* @SuppressWarnings(PHPMD.UnusedFormalParameter) | ||
*/ | ||
public function test($identifier) | ||
{ | ||
// TODO: Implement test() method. | ||
} | ||
|
||
/** | ||
* Load cache record by its unique identifier | ||
* | ||
* @param string $identifier | ||
* @return string|bool | ||
* @SuppressWarnings(PHPMD.UnusedFormalParameter) | ||
*/ | ||
public function load($identifier) | ||
{ | ||
return $this->cachePath ? @include($this->getCachePath($identifier)) : false; | ||
} | ||
|
||
/** | ||
* Save cache record | ||
* | ||
* @param string $data | ||
* @param string $identifier | ||
* @param array $tags | ||
* @param int|bool|null $lifeTime | ||
* @return bool | ||
* @SuppressWarnings(PHPMD.UnusedFormalParameter) | ||
*/ | ||
public function save($data, $identifier, array $tags = [], $lifeTime = null) | ||
{ | ||
if ($this->cachePath) { | ||
$path = $this->getCachePath($identifier); | ||
if (!is_dir(dirname($path))) { | ||
mkdir(dirname($path)); | ||
} | ||
file_put_contents( | ||
$path, | ||
'<?php return ' . var_export($data, true) . '?>' | ||
); | ||
} | ||
} | ||
|
||
/** | ||
* Remove cache record by its unique identifier | ||
* | ||
* @param string $identifier | ||
* @return bool | ||
* @SuppressWarnings(PHPMD.UnusedFormalParameter) | ||
*/ | ||
public function remove($identifier) | ||
{ | ||
// TODO: Implement remove() method. | ||
} | ||
|
||
/** | ||
* Clean cache records matching specified tags | ||
* | ||
* @param string $mode | ||
* @param array $tags | ||
* @return bool | ||
* @SuppressWarnings(PHPMD.UnusedFormalParameter) | ||
*/ | ||
public function clean($mode = \Zend_Cache::CLEANING_MODE_ALL, array $tags = []) | ||
joni-jones marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
// TODO: Implement clean() method. | ||
} | ||
|
||
/** | ||
* Retrieve backend instance | ||
* | ||
* @return \Zend_Cache_Backend_Interface | ||
*/ | ||
public function getBackend() | ||
{ | ||
// TODO: Implement getBackend() method. | ||
} | ||
|
||
/** | ||
* Retrieve frontend instance compatible with Zend Locale Data setCache() to be used as a workaround | ||
* | ||
* @return \Zend_Cache_Core | ||
*/ | ||
public function getLowLevelFrontend() | ||
{ | ||
// TODO: Implement getLowLevelFrontend() method. | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
namespace Magento\Framework\CompiledInterception\Generator; | ||
|
||
use Magento\Framework\Serialize\SerializerInterface; | ||
|
||
class NoSerialize implements SerializerInterface | ||
{ | ||
/** | ||
* Serialize data into string | ||
* | ||
* @param string|int|float|bool|array|null $data | ||
* @return string|bool | ||
* @throws \InvalidArgumentException | ||
* @since 101.0.0 | ||
*/ | ||
public function serialize($data) | ||
{ | ||
return $data; | ||
} | ||
|
||
/** | ||
* Unserialize the given string | ||
* | ||
* @param string $string | ||
* @return string|int|float|bool|array|null | ||
* @throws \InvalidArgumentException | ||
* @since 101.0.0 | ||
*/ | ||
public function unserialize($string) | ||
{ | ||
return $string; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
namespace Magento\Framework\CompiledInterception\Generator; | ||
|
||
use Magento\Framework\Config\ScopeInterface; | ||
|
||
class StaticScope implements ScopeInterface | ||
{ | ||
/** | ||
* @var string | ||
*/ | ||
protected $scope; | ||
engcom-Foxtrot marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
public function __construct($scope) | ||
{ | ||
$this->scope = $scope; | ||
} | ||
|
||
/** | ||
* Get current configuration scope identifier | ||
* | ||
* @return string | ||
*/ | ||
public function getCurrentScope() { | ||
return $this->scope; | ||
} | ||
|
||
/** | ||
* @param string $scope | ||
* @throws \Exception | ||
* @SuppressWarnings(PHPMD.UnusedFormalParameter) | ||
*/ | ||
public function setCurrentScope($scope){ | ||
throw new \Exception('readonly'); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
### ABOUT | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please, make sure the latest README is updated according to the latest changes. |
||
|
||
This component changes the way Magento 2 generates Interceptor classes (a mechanism that allows plugins to work together). | ||
|
||
Instead of generating boilerplate code it compiles the Interceptor using information from source code. | ||
This makes plugins slightly faster and as Magento uses a lot of plugins, even at its core, it lowers request time by ~10%. | ||
This is important in places where there is a lot of non-cached PHP logic going on (for example admin panel is noticeably faster). | ||
|
||
The default method uses code that is called on nearly each method to see if there are any plugins connected, in generated code this is not required and call-stack is reduced. | ||
|
||
Having plugins called directly also makes code easier to debug and bugs easier to find. | ||
|
||
The Interceptors generated by this plugin are 100% compatible with the ones generated by Magento by default, so there is no need to change anything in your plugins. | ||
|
||
### ENABLING | ||
|
||
To use compiled interceptors in developer mode please add following preference to your di.xml | ||
``` | ||
<preference for="Magento\Framework\Interception\Code\Generator\Interceptor" type="Magento\Framework\CompiledInterception\Generator\CompiledInterceptor" /> | ||
``` | ||
|
||
To use compiled interceptors in production mode please add following preference to your di.xml | ||
fsw marked this conversation as resolved.
Show resolved
Hide resolved
|
||
``` | ||
<preference for="Magento\Setup\Module\Di\Code\Generator\Interceptor" type="Magento\Framework\CompiledInterception\Generator\CompiledInterceptor" /> | ||
``` | ||
|
||
Clear generated files and cache: | ||
|
||
`rm -rf generated/* && rm -rf var/cache/* && bin/magento cache:clean` | ||
|
||
### DISABLING | ||
|
||
Remove preferences from `app/etc/di.xml`, remove module and clear cache and generated files. | ||
|
||
### TECHNICAL DETAILS | ||
|
||
Instead of interceptors that read plugins config at runtime like this: | ||
|
||
``` | ||
public function methodX($arg) { | ||
$pluginInfo = $this->pluginList->getNext($this->subjectType, 'methodX'); | ||
if (!$pluginInfo) { | ||
return parent::methodX($arg); | ||
} else { | ||
return $this->___callPlugins('methodX', func_get_args(), $pluginInfo); | ||
} | ||
} | ||
``` | ||
|
||
This generator generates static interceptors like this: | ||
|
||
|
||
``` | ||
public function methodX($arg) { | ||
switch(getCurrentScope()){ | ||
case 'frontend': | ||
$this->_get_example_plugin()->beforeMethodX($this, $arg); | ||
$this->_get_another_plugin()->beforeMethodX($this, $arg); | ||
$result = $this->_get_around_plugin()->aroundMethodX($this, function($arg){ | ||
return parent::methodX($arg); | ||
}); | ||
return $this->_get_after_plugin()->afterMethodX($this, $result); | ||
case 'adminhtml': | ||
// ... | ||
default: | ||
return parent::methodX($arg); | ||
} | ||
} | ||
``` | ||
|
||
|
||
#### PROS | ||
|
||
* Easier debugging. | ||
* If you ever stumbled upon `___callPlugins` when debugging you should know how painful it is to debug issues inside plugins. | ||
* Generated code is decorated with PHPDoc for easier debugging in IDE | ||
|
||
* Fastest response time (5%-15% faster in developer and production mode) | ||
* No redundant calls to `___callPlugins` in call stack. | ||
* Methods with no plugins are not overridden in parent at all. | ||
|
||
* Implemented as a module and can be easily reverted to the default `Generator\Interceptor` | ||
|
||
#### CONS | ||
|
||
* Each time after making change in etc plugins config, `generated/code/*` needs to be purged | ||
* Tiny longer code generation step when run with no cache | ||
* As this does not load plugins at runtime, might not work in an edge case of plugging into core Magento classes like `PluginsList` etc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to inherit from PluginList,
just implement the interface
\Magento\Framework\Interception\PluginListInterface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This inherits from
PluginList
not to duplicate code. It uses parent class logic to make sure plugins will be executed exactly in the same order and using same logic as they were previously.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you inherit from PluginList then you have to implement anything which is private.
Properties: $logger, $serializer
Methods: trimInstanceStartingBackslash, filterPlugins, getLogger
Make life easier for anyone who wants to subclass from you and define them as protected. You are allowed to raise the visibility of a private method in a child since the parent and any siblings don't know about it.