[GitHub] spark pull request: [SPARK-3679] [PySpark] pickle the exact global...

2014-09-24 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/2522


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-3679] [PySpark] pickle the exact global...

2014-09-24 Thread JoshRosen
Github user JoshRosen commented on the pull request:

https://github.com/apache/spark/pull/2522#issuecomment-56729295
  
LGTM; merging this now.  Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-3679] [PySpark] pickle the exact global...

2014-09-24 Thread davies
Github user davies commented on a diff in the pull request:

https://github.com/apache/spark/pull/2522#discussion_r17995491
  
--- Diff: python/pyspark/cloudpickle.py ---
@@ -304,16 +313,37 @@ def save_function_tuple(self, func, forced_imports):
 write(pickle.REDUCE)  # applies _fill_function on the tuple
 
 @staticmethod
-def extract_code_globals(code):
+def extract_code_globals(co):
 """
 Find all globals names read or written to by codeblock co
 """
-names = set(code.co_names)
-if code.co_consts:   # see if nested function have any global refs
-for const in code.co_consts:
+code = co.co_code
+names = co.co_names
+out_names = set()
+
+n = len(code)
+i = 0
+extended_arg = 0
+while i < n:
+op = code[i]
+
+i = i+1
+if op >= HAVE_ARGUMENT:
+oparg = ord(code[i]) + ord(code[i+1])*256 + extended_arg
+extended_arg = 0
+i = i+2
+if op == EXTENDED_ARG:
+extended_arg = oparg*65536L
+if op in GLOBAL_OPS:
+out_names.add(names[oparg])
+#print 'extracted', out_names, ' from ', names
+
+if co.co_consts:   # see if nested function have any global refs
--- End diff --

The original code only perform two levels of functions, the new version can 
handle multiple levels.

Each level is on function code, which is created by `def` or `lambda`, so I 
think the code cannot be recursive.

 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-3679] [PySpark] pickle the exact global...

2014-09-24 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/2522#issuecomment-56726780
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20759/


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-3679] [PySpark] pickle the exact global...

2014-09-24 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/2522#issuecomment-56726767
  
  [QA tests have 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/20759/consoleFull)
 for   PR 2522 at commit 
[`dfbccf5`](https://github.com/apache/spark/commit/dfbccf5c92333da8ab835fc4730aadc844e9f895).
 * This patch **passes** unit tests.
 * This patch merges cleanly.
 * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-3679] [PySpark] pickle the exact global...

2014-09-24 Thread JoshRosen
Github user JoshRosen commented on the pull request:

https://github.com/apache/spark/pull/2522#issuecomment-56726010
  
This looks good to me, pending Jenkins.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-3679] [PySpark] pickle the exact global...

2014-09-24 Thread JoshRosen
Github user JoshRosen commented on a diff in the pull request:

https://github.com/apache/spark/pull/2522#discussion_r17994793
  
--- Diff: python/pyspark/cloudpickle.py ---
@@ -304,16 +313,37 @@ def save_function_tuple(self, func, forced_imports):
 write(pickle.REDUCE)  # applies _fill_function on the tuple
 
 @staticmethod
-def extract_code_globals(code):
+def extract_code_globals(co):
 """
 Find all globals names read or written to by codeblock co
 """
-names = set(code.co_names)
-if code.co_consts:   # see if nested function have any global refs
-for const in code.co_consts:
+code = co.co_code
+names = co.co_names
+out_names = set()
+
+n = len(code)
+i = 0
+extended_arg = 0
+while i < n:
+op = code[i]
+
+i = i+1
+if op >= HAVE_ARGUMENT:
+oparg = ord(code[i]) + ord(code[i+1])*256 + extended_arg
+extended_arg = 0
+i = i+2
+if op == EXTENDED_ARG:
+extended_arg = oparg*65536L
+if op in GLOBAL_OPS:
+out_names.add(names[oparg])
+#print 'extracted', out_names, ' from ', names
+
+if co.co_consts:   # see if nested function have any global refs
--- End diff --

Based on a read through this PR, it looks like this line is the first place 
where this function diverges from the pre-#2144 version of `cloudpickle`.

It looks like the original version of `cloudpickle` called this code from 
outside of  `extract_code_globals`, so I guess the old code would only perform 
one level of recursion when trying to extract globals?

Do you think that adding actual, unbounded recursion could cause problems 
here?  If the "nested function" implies that this only applies to functions 
defined within other functions, then there aren't cycles in the nesting and 
therefore shouldn't be cycles that lead to infinite recursion.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-3679] [PySpark] pickle the exact global...

2014-09-24 Thread davies
Github user davies commented on the pull request:

https://github.com/apache/spark/pull/2522#issuecomment-56725936
  
@JoshRosen fixed the description


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-3679] [PySpark] pickle the exact global...

2014-09-24 Thread JoshRosen
Github user JoshRosen commented on the pull request:

https://github.com/apache/spark/pull/2522#issuecomment-56724946
  
Note for other reviewers: this commit mostly reverts #2144's changes to 
`extract_code_globals`, so although the changed code is complicated, we were 
using it before in `cloudpickle` and should be able to trust that it works.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-3679] [PySpark] pickle the exact global...

2014-09-24 Thread JoshRosen
Github user JoshRosen commented on the pull request:

https://github.com/apache/spark/pull/2522#issuecomment-56724555
  
Based on the JIRA, it looks like this PR description should link to #2144 
instead?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-3679] [PySpark] pickle the exact global...

2014-09-24 Thread SparkQA
Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/2522#issuecomment-56717278
  
  [QA tests have 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/20759/consoleFull)
 for   PR 2522 at commit 
[`dfbccf5`](https://github.com/apache/spark/commit/dfbccf5c92333da8ab835fc4730aadc844e9f895).
 * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request: [SPARK-3679] [PySpark] pickle the exact global...

2014-09-24 Thread davies
GitHub user davies opened a pull request:

https://github.com/apache/spark/pull/2522

[SPARK-3679] [PySpark] pickle the exact globals of functions

function.func_code.co_names has all the names used in the function, 
including name of attributes. It will pickle some unnecessary globals if there 
is a global having the same name with attribute (in co_names).

There is a regression introduced by #2114 

cc @JoshRosen 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/davies/spark globals

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/2522.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2522


commit dfbccf5c92333da8ab835fc4730aadc844e9f895
Author: Davies Liu 
Date:   2014-09-24T18:23:10Z

fix bug while pickle globals of function




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org