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 2020/01/16 23:36:30 UTC
[GitHub] [druid] suneet-s opened a new pull request #9200: Optimize
JoinCondition matching
suneet-s opened a new pull request #9200: Optimize JoinCondition matching
URL: https://github.com/apache/druid/pull/9200
### Description
The LookupJoinMatcher needs to check if a condition is always true or false
multiple times. This can be pre-computed to speed up the match checking
This change reduces the time it takes to perform a for joining on a long key
from ~ 36 ms/op to 23 ms/ op
This PR has:
- [ ] been self-reviewed.
- [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] added documentation for new or modified features or behaviors.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/licenses.yaml)
- [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on a change in pull request #9200:
Optimize JoinCondition matching
Posted by GitBox <gi...@apache.org>.
suneet-s commented on a change in pull request #9200: Optimize JoinCondition matching
URL: https://github.com/apache/druid/pull/9200#discussion_r368080896
##########
File path: processing/src/main/java/org/apache/druid/segment/join/JoinConditionAnalysis.java
##########
@@ -133,26 +142,23 @@ public String getOriginalExpression()
*/
public boolean isAlwaysFalse()
{
- return nonEquiConditions.stream()
- .anyMatch(expr -> expr.isLiteral() && !expr.eval(ExprUtils.nilBindings()).asBoolean());
+ return anyFalseLiteralNonEquiConditions;
}
/**
* Return whether this condition is a constant that is always true.
*/
public boolean isAlwaysTrue()
{
- return equiConditions.isEmpty() &&
- nonEquiConditions.stream()
- .allMatch(expr -> expr.isLiteral() && expr.eval(ExprUtils.nilBindings()).asBoolean());
+ return equiConditions.isEmpty() && allTrueLiteralNonEquiConditions;
Review comment:
Done
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9200: Optimize
JoinCondition matching
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9200: Optimize JoinCondition matching
URL: https://github.com/apache/druid/pull/9200#discussion_r367719384
##########
File path: processing/src/main/java/org/apache/druid/segment/join/JoinConditionAnalysis.java
##########
@@ -133,26 +142,23 @@ public String getOriginalExpression()
*/
public boolean isAlwaysFalse()
{
- return nonEquiConditions.stream()
- .anyMatch(expr -> expr.isLiteral() && !expr.eval(ExprUtils.nilBindings()).asBoolean());
+ return anyFalseLiteralNonEquiConditions;
}
/**
* Return whether this condition is a constant that is always true.
*/
public boolean isAlwaysTrue()
{
- return equiConditions.isEmpty() &&
- nonEquiConditions.stream()
- .allMatch(expr -> expr.isLiteral() && expr.eval(ExprUtils.nilBindings()).asBoolean());
+ return equiConditions.isEmpty() && allTrueLiteralNonEquiConditions;
Review comment:
It seems like `allTrueLiteralNonEquiConditions` is only used here; how about caching `isAlwaysTrue` directly?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9200: Optimize
JoinCondition matching
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9200: Optimize JoinCondition matching
URL: https://github.com/apache/druid/pull/9200#discussion_r367719499
##########
File path: processing/src/main/java/org/apache/druid/segment/join/JoinConditionAnalysis.java
##########
@@ -133,26 +142,23 @@ public String getOriginalExpression()
*/
public boolean isAlwaysFalse()
{
- return nonEquiConditions.stream()
- .anyMatch(expr -> expr.isLiteral() && !expr.eval(ExprUtils.nilBindings()).asBoolean());
+ return anyFalseLiteralNonEquiConditions;
Review comment:
Why not call this `isAlwaysFalse`? (It looks like it isn't used anywhere else, and it seems to me to be easier to understand the meaning of the field if it's named after what we want it to mean.)
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on issue #9200: Optimize JoinCondition
matching
Posted by GitBox <gi...@apache.org>.
suneet-s commented on issue #9200: Optimize JoinCondition matching
URL: https://github.com/apache/druid/pull/9200#issuecomment-575752258
> Cool, thank you for the PR. It seems like the benchmark class is not in master yet. Would you please link the benchmark class here?
@jihoonson here's the benchmark I was using. I will push it up to master once I clean it up a little more and add more tests
https://github.com/gianm/druid/blob/joins/benchmarks/src/main/java/org/apache/druid/benchmark/JoinAndLookupBenchmark.java
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] fjy merged pull request #9200: Optimize JoinCondition
matching
Posted by GitBox <gi...@apache.org>.
fjy merged pull request #9200: Optimize JoinCondition matching
URL: https://github.com/apache/druid/pull/9200
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on a change in pull request #9200:
Optimize JoinCondition matching
Posted by GitBox <gi...@apache.org>.
suneet-s commented on a change in pull request #9200: Optimize JoinCondition matching
URL: https://github.com/apache/druid/pull/9200#discussion_r368080871
##########
File path: processing/src/main/java/org/apache/druid/segment/join/JoinConditionAnalysis.java
##########
@@ -133,26 +142,23 @@ public String getOriginalExpression()
*/
public boolean isAlwaysFalse()
{
- return nonEquiConditions.stream()
- .anyMatch(expr -> expr.isLiteral() && !expr.eval(ExprUtils.nilBindings()).asBoolean());
+ return anyFalseLiteralNonEquiConditions;
Review comment:
yeah. I wasn't sure if isAlwaysFalse could be more complex going forward. This is clearer - it took me a while to wrap my head around what isAlwaysFalse was trying to check. Added comments, and used `isAlways...`
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org