You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/03/28 19:37:59 UTC
[GitHub] [druid] jihoonson opened a new pull request #11041: Add explicit EOF and use assert instead of exception
jihoonson opened a new pull request #11041:
URL: https://github.com/apache/druid/pull/11041
### Description
This PR fixes 2 issues.
- Implicit EOF in the Antlr grammar file. This makes Antlr parser to think that something might be wrong when it sees EOF and [creates a `NoViableAltException` instance](https://github.com/antlr/antlr4/blob/master/runtime/Java/src/org/antlr/v4/runtime/atn/ParserATNSimulator.java#L467). This could be problematic for queries that Calcite needs to explore large space of candidate queries as the Antlr parser will create exceptions over again even though those exceptions are ignored. The below flame graph shows initializing exceptions was one of the most expensive parts for [the query 19 (giant union query) in SqlBenchmark](https://github.com/apache/druid/blob/master/benchmarks/src/test/java/org/apache/druid/benchmark/query/SqlBenchmark.java#L152). [Implicit EOF can also cause `NoViableAltException` to be thrown incorrectly sometimes](https://github.com/antlr/antlr4/issues/118). This PR adds explicit EOF in the grammar file.
- `getClass().getSimpleName()` in `BottomUpTransform.checkedProcess()` can be expensive if it is called lots of times. This PR changes it to use `assert`.
`SqlBenchmark.planSql()` results.
```
master
Benchmark (query) (rowsPerSegment) (vectorize) Mode Cnt Score Error Units
SqlBenchmark.planSql 0 5 false avgt 15 0.563 ± 0.005 ms/op
SqlBenchmark.planSql 1 5 false avgt 15 0.603 ± 0.004 ms/op
SqlBenchmark.planSql 2 5 false avgt 15 0.662 ± 0.007 ms/op
SqlBenchmark.planSql 3 5 false avgt 15 0.808 ± 0.009 ms/op
SqlBenchmark.planSql 4 5 false avgt 15 1.156 ± 0.014 ms/op
SqlBenchmark.planSql 5 5 false avgt 15 1.291 ± 0.011 ms/op
SqlBenchmark.planSql 6 5 false avgt 15 1.504 ± 0.011 ms/op
SqlBenchmark.planSql 7 5 false avgt 15 1.270 ± 0.013 ms/op
SqlBenchmark.planSql 8 5 false avgt 15 2.044 ± 0.013 ms/op
SqlBenchmark.planSql 9 5 false avgt 15 1.982 ± 0.022 ms/op
SqlBenchmark.planSql 10 5 false avgt 15 0.707 ± 0.005 ms/op
SqlBenchmark.planSql 11 5 false avgt 15 0.741 ± 0.002 ms/op
SqlBenchmark.planSql 12 5 false avgt 15 0.624 ± 0.006 ms/op
SqlBenchmark.planSql 13 5 false avgt 15 0.874 ± 0.007 ms/op
SqlBenchmark.planSql 14 5 false avgt 15 0.980 ± 0.008 ms/op
SqlBenchmark.planSql 15 5 false avgt 15 0.628 ± 0.007 ms/op
SqlBenchmark.planSql 16 5 false avgt 15 0.745 ± 0.005 ms/op
SqlBenchmark.planSql 17 5 false avgt 15 0.956 ± 0.009 ms/op
SqlBenchmark.planSql 18 5 false avgt 15 1.078 ± 0.011 ms/op
SqlBenchmark.planSql 19 5 false avgt 15 129.221 ± 0.725 ms/op
patch
Benchmark (query) (rowsPerSegment) (vectorize) Mode Cnt Score Error Units
SqlBenchmark.planSql 0 5 force avgt 15 0.553 ± 0.007 ms/op
SqlBenchmark.planSql 1 5 force avgt 15 0.610 ± 0.006 ms/op
SqlBenchmark.planSql 2 5 force avgt 15 0.669 ± 0.005 ms/op
SqlBenchmark.planSql 3 5 force avgt 15 0.768 ± 0.009 ms/op
SqlBenchmark.planSql 4 5 force avgt 15 1.136 ± 0.010 ms/op
SqlBenchmark.planSql 5 5 force avgt 15 1.151 ± 0.012 ms/op
SqlBenchmark.planSql 6 5 force avgt 15 1.337 ± 0.011 ms/op
SqlBenchmark.planSql 7 5 force avgt 15 1.184 ± 0.008 ms/op
SqlBenchmark.planSql 8 5 force avgt 15 1.838 ± 0.016 ms/op
SqlBenchmark.planSql 9 5 force avgt 15 1.699 ± 0.018 ms/op
SqlBenchmark.planSql 10 5 force avgt 15 0.710 ± 0.005 ms/op
SqlBenchmark.planSql 11 5 force avgt 15 0.738 ± 0.007 ms/op
SqlBenchmark.planSql 12 5 force avgt 15 0.592 ± 0.002 ms/op
SqlBenchmark.planSql 13 5 force avgt 15 0.810 ± 0.006 ms/op
SqlBenchmark.planSql 14 5 force avgt 15 0.906 ± 0.005 ms/op
SqlBenchmark.planSql 15 5 force avgt 15 0.593 ± 0.005 ms/op
SqlBenchmark.planSql 16 5 force avgt 15 0.700 ± 0.006 ms/op
SqlBenchmark.planSql 17 5 force avgt 15 0.853 ± 0.006 ms/op
SqlBenchmark.planSql 18 5 force avgt 15 0.967 ± 0.004 ms/op
SqlBenchmark.planSql 19 5 force avgt 15 105.288 ± 0.724 ms/op
```
<hr>
##### Key changed/added classes in this PR
* Expr.g4`
* `BottomUpTransform`
<hr>
This PR has:
- [x] been self-reviewed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jihoonson merged pull request #11041: Add explicit EOF for expression parser and use assert instead of exception in sql planner
Posted by GitBox <gi...@apache.org>.
jihoonson merged pull request #11041:
URL: https://github.com/apache/druid/pull/11041
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org