[REF] improve XPath expressions, compile them only once, fix false negative #79
Conversation
Added simple profiling test cases to get insights into the program flow and possible bottlenecks of the system. The profiling can be done on user-repositories with the following command: `PROFILING_TEST_REPO=path/to/repo tox -e cprofile`
…gative * Some XPath expressions were improved to offload work from python code * Similar to regex, xpath expressions are compiled at the class level and reused by checks, this means they are only compiled once instead of multiple times. More info:https://lxml.de/xpathxslt.html#the-xpath-class * There was a false negative in test_repo/broken_module/skip_xml_check2.xml:3, xml-data-deprecated-node should be raised but was not, this was discovered when improving the xpath expressions. Apparently the previous method (using iterchildren) also counted comments in.
45782e1 to
1d3cb60
Compare
|
Initially I was just going to add cprofile tests to get a baseline idea of the state of the program. I don't think optimization is a priority right now but implementing the tests was simple and did not hurt. I then found out XPath expressions could be compiled and used the approach out of curiosity to see if there were any perf improvements. My observations show a 5-10% improvement in program run time. During the POC I found out some xpath expressions could be improved to reduce the need for python processing and also fixed a false negative (I think) so I thought the changes were helpful enough to grant an MR. Can you review @moylop260 @luisg123v |
|
Interesting PR FYI I had created a similar PR for Pylint a long time ago, but it was not merged: Thank you! Could you review it, please? |
| @@ -11,6 +11,35 @@ | |||
|
|
|||
|
|
|||
| class ChecksOdooModuleXML: | |||
| xpath_deprecated_data = etree.XPath("/odoo[count(./*) < 2]/data|/openerp[count(./*) < 2]/data") | |||
| xpath_view_replaces = etree.XPath( | |||
| ".//field[@name='name' and @position='replace'][1] | .//*[@position='replace'][1]" | |||
|
@moylop260 It looks you didn't consider #78 as I commented on #78 (review) |
@antonag32 vas wey |
Removed a redundant term in one of the xpath expressions and refactored code from PR OCA#79 to reuse the same xpath expression.
Removed a redundant term in one of the xpath expressions and refactored code from PR #79 to reuse the same xpath expression.
Added simple profiling test cases to get insights into the program
flow and possible bottlenecks of the system. The profiling
can be done on user-repositories with the following command:
PROFILING_TEST_REPO=path/to/repo tox -e cprofileSome XPath expressions were improved to offload work from python code
Similar to regex, xpath expressions are compiled at the class level and
reused by checks, this means they are only compiled once instead of
multiple times. More info:https://lxml.de/xpathxslt.html#the-xpath-class
There was a false negative in test_repo/broken_module/skip_xml_check2.xml:3,
xml-data-deprecated-node should be raised but was not, this was discovered
when improving the xpath expressions. Apparently the previous method
(using iterchildren) also counted comments in.