Repository navigation
dramatic performance degradation between JSqlParser 4.0 and 4.2 #1397
Description
Some of our included PRs had a dramatic performance impact. So they need to be revisited. This log comes from the same SQL which uses non recursive large case when statements.
JSqlParser 4.2
466091 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_SpecialStringFunctionWithNamedParameters_4486_9_249())
869250 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_Column_1597_5_134())
870821 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_UserVariable_3831_5_215())
870821 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_UserVariable_3831_12_456())
1335347 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_RelObjectNameList_1584_5_323())
1587076 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_RelObjectName_1656_5_174())
1587076 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_RelObjectNameExt_1686_7_409())
1587076 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_RelObjectName_1656_6_396())
1587076 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_RelObjectNameExt_1686_5_189())
1587076 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_RelObjectNameWithoutValue_1612_5_601())
177987285 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_scan_token(int))
JSqlParser 4.0
75789 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3_28())
75995 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_Column_1307_5_114())
76148 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_UserVariable_3334_5_175())
76148 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_UserVariable_3334_12_358())
116525 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_RelObjectNameList_1294_5_226())
154419 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_RelObjectName_1350_5_133())
154419 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_RelObjectName_1350_6_295())
154419 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_RelObjectNameExt_1380_5_147())
154419 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_RelObjectNameWithoutValue_1322_5_415())
154419 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_3R_RelObjectNameExt_1380_7_308())
9681645 - execution(private boolean net.sf.jsqlparser.parser.CCJSqlParser.jj_scan_token(int))
If you look at the methods call log, some methods are called 10 times and more of the version 4.0.
Maybe we have to rewind some PRs for this.
Activity
wumpz commentedon Nov 2, 2021
@manticore-projects We have to check those complex expression extension for case when constructs. I have a performance degradation of 10x and more here. At least I suspect those changes to be the problem.
manticore-projects commentedon Nov 2, 2021
Can you give me the particular SQL statements please, which you have used for the test and maybe point out how exactly you measured? Should we involve PMD as Test Dependency to measure impact of changes more reliably?
(Right now I can only use the "total build time" to estimate the impact of a certain change.)
Also, we would need to discuss the objectives "correctness" vs. "performance". In my own opinion, a parser must be as correct as possible at first, and only after that should be as fast as possible. I was reluctant to sacrifice correctness over performance. But of course we should look into optimizations, when a particular construct causes such performance deterioration.
wumpz commentedon Nov 2, 2021
Sure, but a degradation of 10x is huge. I am working on an example SQL. For statistics of this I wrote https://github.com/JSQLParser/javacc-perf-diag.
manticore-projects commentedon Nov 4, 2021
I was not aware, that there is another Performance related Test Suite aside our normal integration tests (with defined 2sec timeouts).
So far I was satisfied when the normal tests ran through, within 30 seconds in total and without a time out exception for a single test. (Just explaining where I come from.)
Of course I share your concern about the performance deterioration. In my opinion, it's a two step:
we need a tool, which can run benchmarks across all GIT commits in order to establish a) a benchmark and b) identify a a commit which breaks a certain threshold (e.g. more than 20% slower) -- best case with identification of the statement
when we have identified the commit and the statement, we can start to break our heads how we can get it faster
I believe investing into step 1 makes sense because it will be a re-occurring problem: Correctness unfortunately competes with performance and every single improvement on the Correctness will jeopardize the performance.
So we would need to run this benchmarking tool before we commit anything and make it part of the CI process.
It also means, that we should find a smart way to extract all SQL statements from the Test Suite. Currently they are burried inside the Java Code and not easy to access from outside.
chensk0601 commentedon Nov 15, 2021
any plans to fix this issue? I meet the same questions too.
manticore-projects commentedon Nov 15, 2021
With what query and which version on JSQLParser exactly please.
wumpz commentedon Nov 15, 2021
@chensk0601 The investigation is in progress ... I am right now collecting SQLs that have performance issues for version 4.2 and had no performance issues in version v4 and below. If you have some, feel free to add those at this issue.
wumpz commentedon Nov 16, 2021
Here is one SQL. The problem is, that parsing this using JSqlParser 4.2 uses nearly 10 times of token reads, lookaheads than version 4.0.
sanitized_biq_query_final.txt
Sure there could be a smaller example, but this is the start.
manticore-projects commentedon Nov 16, 2021
Properly formatted version of this beast
Performance Improvements
Performance Improvements (#1439)
chensk0601 commentedon Apr 14, 2022