Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
Thank you very much for your support and merging this. I will open a
follow-up PR soon.
---
-
To unsubscribe, e-mail:
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/19811
great, merging to master, thank you all!
We can address other minor comments in follow-ups
---
-
To unsubscribe,
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85107/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #85107 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85107/testReport)**
for PR 19811 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #85107 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85107/testReport)**
for PR 19811 at commit
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/19811
LGTM with one minor comment.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/19811
LGTM too, thanks @kiszk and @bdrillard! This is a very important PR IMHO
---
-
To unsubscribe, e-mail:
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/19811
LGTM except some minor comments, great job!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
ping @cloud-fan
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19811
I have to revert that PR again.
https://github.com/apache/spark/commit/e58f275678fb4f904124a4a2a1762f04c835eb0e
I think it is caused by that PR. Thus, I revert it again. Although I am
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
[Another
PR](https://github.com/apache/spark/pull/19990#issuecomment-352055032) also
causes the same failure while that PR just added new tests.
---
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
I think this failure due to R package issue that we have seen before.
```
* building 'SparkR_2.3.0.tar.gz'
+ find pkg/vignettes/. -not -name . -not -name '*.Rmd' -not -name '*.md'
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test FAILed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84962/
Test FAILed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84962 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84962/testReport)**
for PR 19811 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84962 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84962/testReport)**
for PR 19811 at commit
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/19811
For some types rarely seen, I think we don't need to make them in array for
now because we might have just few elements there.
---
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
ping @cloud-fan
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84919/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84919 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84919/testReport)**
for PR 19811 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84919 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84919/testReport)**
for PR 19811 at commit
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
Another use case is for frequently-executed variable (e.g. row variable)
for performance. We want to keep it as non-array variable.
---
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/19811
My major concern is how to reason about when we need to force inline. I can
understand the use case of keeping the variable name unchanged(e.g.
`MapObjects`), but what about other cases?
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84908/
Test FAILed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test FAILed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84908 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84908/testReport)**
for PR 19811 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84908 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84908/testReport)**
for PR 19811 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test FAILed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84897 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84897/testReport)**
for PR 19811 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84897/
Test FAILed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84897 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84897/testReport)**
for PR 19811 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test FAILed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84895/
Test FAILed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84895 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84895/testReport)**
for PR 19811 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84895 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84895/testReport)**
for PR 19811 at commit
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
Just resolved conflicts. Next commit will address review comments.
---
-
To unsubscribe, e-mail:
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
@cloud-fan @viirya @maropu @mgaido91 could you please review again? I think
that I addressed all of comments and fixed failures.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84876/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84876 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84876/testReport)**
for PR 19811 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84876 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84876/testReport)**
for PR 19811 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test FAILed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84841/
Test FAILed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84841 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84841/testReport)**
for PR 19811 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84841 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84841/testReport)**
for PR 19811 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84834/
Test FAILed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test FAILed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84834 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84834/testReport)**
for PR 19811 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84834 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84834/testReport)**
for PR 19811 at commit
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
Jenkins, retest this please
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84830/
Test FAILed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84830 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84830/testReport)**
for PR 19811 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test FAILed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84830 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84830/testReport)**
for PR 19811 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84818/
Test PASSed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84818 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84818/testReport)**
for PR 19811 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84818 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84818/testReport)**
for PR 19811 at commit
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
@cloud-fan @gatorsmile I think that it is hard to create an end-to-end test
for initialization with values and splitting arrays with the fixed parameters
in `CodegenContext`.
Is it better to
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
If I remember correctly, this style of adding an option is not recommended
by @gatorsmile.
I will find his comment in the past discussion of PR.
---
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/19811
I didn't mean to add these parameters to `SQLConf`. I just meant to add
test-only parameters for that like;
```
// `spark.sql.codegen.outer_class_variables_threshol` is for testing only.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
IIUC, it is hard to make these value configurable due to [this
reason](https://github.com/apache/spark/pull/19449#discussion_r143291724).
---
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/19811
How about making `OUTER_CLASS_VARIABLES_THRESHOLD ` and
`MUTABLESTATEARRAY_SIZE_LIMIT ` configurable and then testing more for
initialization code, multiple array cases, and so on?
---
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
@maropu Sorry, I overlooked this comment. Good point, however since we
eliminated many mutable states, I think that it is not easy to create
end-to-end test to generation the initialization code for
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84772/
Test PASSed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84772 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84772/testReport)**
for PR 19811 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84772 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84772/testReport)**
for PR 19811 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84761/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84761 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84761/testReport)**
for PR 19811 at commit
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/19811
Can you add a test to generate the initialization code for the compacted
states? IIUC the current tests in this pr don't have the test.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84761 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84761/testReport)**
for PR 19811 at commit
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
@cloud-fan Could you please review this?
I will address an argument issue in another PR after merging this.
---
-
To
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
I think this failure due to package issue, not due to this PR
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84748/
Test FAILed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test FAILed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84748 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84748/testReport)**
for PR 19811 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84748 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84748/testReport)**
for PR 19811 at commit
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
Since we gave up automatic conservation detection as you suggested, it
would be a reasonable way since there are about 15 places to be considers in
the problem while there are more than 100
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/19811
@kiszk I see, thanks. Wouldn't be better anyway to store all the
initialization in the same array so that we ensure that the ordering is the
same as before this patch?
---
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
Good question, got it. There are some cases that we have to ensure order of
declarations.
The current code asks developers to ensure it by using `inline = true`. We
can see [an
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/19811
@kiszk yes, I know, but I meant a different thing. I will try to explain
with an example, let me know if I am not clear enough.
Since we are not defining everything as an array and we are
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/19811
@mgaido91 The latest code does not generate a loop for initializations.
This is an optimization to reduce the JVM bytecode size.
As @cloud-fan pointed out
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/19811
hi @kiszk, have you already checked why there are python failures? I wanted
to share a concern I have with the current implementation. Maybe, it is not a
problem, but I think that currently we are
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84696/
Test FAILed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test FAILed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84696 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84696/testReport)**
for PR 19811 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84696 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84696/testReport)**
for PR 19811 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84638/
Test FAILed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test FAILed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84638 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84638/testReport)**
for PR 19811 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84638 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84638/testReport)**
for PR 19811 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84611/
Test FAILed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test FAILed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19811
**[Test build #84611 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84611/testReport)**
for PR 19811 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Merged build finished. Test FAILed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19811
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84610/
Test FAILed.
---
1 - 100 of 141 matches
Mail list logo
