Re: TpchTest#testQuery07 timeout when running continuous-integration/appveyor/pr

2020-12-04 Thread JiaTao Tao
Hi Julian, Vladimir I remove the equiv rel( *mapRel2Subset.remove(rel)*) in VolcanoPlanner#rename, https://github.com/apache/calcite/pull/2290/files#diff-008c6d52bfd93bbe963a23c264bc412c68cac3b4837e3f10b8d5e4858cd4acb8 : When I run the test again, it throws NPE in RuleQueue#skipMatch, in method R

Re: TpchTest#testQuery07 timeout when running continuous-integration/appveyor/pr

2020-12-03 Thread JiaTao Tao
Hi Julian, Vladimir Actually, I found TpchTest#testQuery07 can directly reproduce the situation. Put this in VolcanoRuleCall#match, delete the previous code, and run the test, we can see the prints are full of the console(about 8k+). I will take a look into this, to see if we can remove the outda

Re: TpchTest#testQuery07 timeout when running continuous-integration/appveyor/pr

2020-12-03 Thread Julian Hyde
I think Vladimir is very likely correct that the performance regression was due to algorithmic changes, that getRelList should not be used, and that improving its performance is not solving the real problem. So, I will not merge a fix for getRelList performance until after the algorithmic problem h

Re: TpchTest#testQuery07 timeout when running continuous-integration/appveyor/pr

2020-12-03 Thread JiaTao Tao
Hi Vladimir I got your opinion, may we do a quick fix about the performance, then I'll take a look again about this. Regards! Aron Tao Vladimir Sitnikov 于2020年12月4日周五 上午5:13写道: The improvement to RelTraitSet.satisfies would be nice, however, I still believe getRelList must not be used in 222

Re: TpchTest#testQuery07 timeout when running continuous-integration/appveyor/pr

2020-12-03 Thread JiaTao Tao
Hi Julian Thanks, Julian, your advice is really helpful! Do you mind I pull a request to merge your code so that we can improve the performance very quickly? About " stored the list of RelNodes ", do you concern about the duplicate "relnode" store in "relsubset"? IMO, what we store is a reference,

Re: TpchTest#testQuery07 timeout when running continuous-integration/appveyor/pr

2020-12-03 Thread Vladimir Sitnikov
The improvement to RelTraitSet.satisfies would be nice, however, I still believe getRelList must not be used in . It looks like the only possibility of the case when rel points to a relset which does not include the rel is the case when the rel has been replaced with an equivalent. In other w

Re: TpchTest#testQuery07 timeout when running continuous-integration/appveyor/pr

2020-12-03 Thread Julian Hyde
A long time ago, we stored the list of RelNodes in each RelSubset, and changed to a shared list in RelSet. I still think it makes sense, especially when subsets are not disjoint (e.g. if a set has one subset sorted by (x) and another subset sorted by (x, y) then all of the RelNodes in the latter wi

Re: TpchTest#testQuery07 timeout when running continuous-integration/appveyor/pr

2020-12-03 Thread JiaTao Tao
Hi Chunwei Sure, I'll organize a document to list the root clauses about the optimization StackOverflow and fire JIRAS, thank for your help. Regards! Aron Tao Chunwei Lei 于2020年12月3日周四 下午4:01写道: > Thank you for your effort, Aron. > > I am wondering if you can file a jira issue and paste your

Re: TpchTest#testQuery07 timeout when running continuous-integration/appveyor/pr

2020-12-03 Thread JiaTao Tao
Hi the flame graph is IDEA's feature, you can directly run with the flame graph. As I said does increase the time, but the time is almost from getRelList, so if we can opt getRelList, the overhead is gone. Regards! Aron Tao Vladimir Sitnikov 于2020年12月3日周四 下午9:02写道: > >From the flame grap

Re: TpchTest#testQuery07 timeout when running continuous-integration/appveyor/pr

2020-12-03 Thread Vladimir Sitnikov
>From the flame graph, the most time is wasted in "getRelList", I didn't see How do you collect and produce the flamegraphs? Do you use debugnonsafepoints Java option? Here are my measurements for running testQuery07: with : 47..57 seconds without : 40..45 seconds I do not know how much

Re: TpchTest#testQuery07 timeout when running continuous-integration/appveyor/pr

2020-12-03 Thread JiaTao Tao
Hi Vladimir >From the flame graph, the most time is wasted in "getRelList", I didn't see "getRelList().contains(...)" takes time, maybe it's not cost performance as we thought, if you do worry about it, I can store RelSubset#rels in Set. Vladimir > It would be great to resolve those cases, howeve

Re: TpchTest#testQuery07 timeout when running continuous-integration/appveyor/pr

2020-12-03 Thread Vladimir Sitnikov
JiaTao Tao, thanks for the details! JiaTao Tao> found. Without pull/, some cases will fail due to "There are not enough JiaTao Tao> rules to produce a node with desired properties: There is 1 empty subset", JiaTao Tao> the root cause is the outdated rule matches have been fired. The comment i

Re: TpchTest#testQuery07 timeout when running continuous-integration/appveyor/pr

2020-12-03 Thread Chunwei Lei
Thank you for your effort, Aron. I am wondering if you can file a jira issue and paste your findings. I would like to look into it if I find some time. Best, Chunwei On Thu, Dec 3, 2020 at 1:15 PM JiaTao Tao wrote: > Hi Vladimir > Thanks for your reply: > 1. I run TpchTest#testQuery07 in my M

Re: TpchTest#testQuery07 timeout when running continuous-integration/appveyor/pr

2020-12-02 Thread JiaTao Tao
Hi Vladimir Thanks for your reply: 1. I run TpchTest#testQuery07 in my MPB, the avg time is 43s, and after re-run the PR ut, the ut is passed, so this ut timeout may be an occasional situation. 2. For the pull/, it due increase the time of TpchTest#testQuery07, about 23%, sorry for that, we te

Re: TpchTest#testQuery07 timeout when running continuous-integration/appveyor/pr

2020-12-02 Thread Vladimir Sitnikov
Hi, I've made a quick check, and the results are as follows: 1) The test was enabled recently (2020-10-20) in https://github.com/apache/calcite/commit/b5a761e559ca1c1c914e388df4c6a0958dc17fc8 2) One of the recent changes that do significantly impact the performance of the test is https://github.co

TpchTest#testQuery07 timeout when running continuous-integration/appveyor/pr

2020-12-02 Thread JiaTao Tao
Hi fellows My PR failed due to, seems an unstable ut, have anyone met this? FAILURE 300.8sec, org.apache.calcite.adapter.tpch.TpchTest > testQuery07() java.util.concurrent.TimeoutException: testQuery07() timed out after 5 minutes https://github.com/apache/calcite/pull/2284 https://ci.appveyo