These are chat archives for dropbox/pyston

11th
May 2016
Kevin Modzelewski
@kmod
May 11 2016 02:04
I'm not sure if this is what you're referring to, but I think the "mutate in-place if refcount is 1" trick is used by more than just numpy, both in Python and in other languages
I think we can solve it so that we don't add extra refs
I was just hoping that we could avoid that work :P
Sun
@Daetalus
May 11 2016 04:58
@njsmith Can module.__dict__ accept not only dict, but alsoattrwrapper if it run with Pyston? @kmod
Nathaniel J. Smith
@njsmith
May 11 2016 06:57
@Daetalus: I don't understand the question
Nathaniel J. Smith
@njsmith
May 11 2016 07:03
@kmod: yeah, probably, I was mostly being cranky about ndarray.resize having a nasty API that I wish we could kill. It directly exposes the refcnt information in the semantics :-/

with the sys.getsizeof() we are now down to:
```
Ran 5469 tests in 24.138s

FAILED (KNOWNFAIL=3, SKIP=10, errors=28, failures=11)
```

^^ was this referring to numpy? O.o
Sun
@Daetalus
May 11 2016 08:10
yes, @njsmith , this is the current NumPy with Pyston status.
Sun
@Daetalus
May 11 2016 08:15

In CPython, module's __dict__ attribute is dict type. Like this:

print(type(__builtins__.__dict__))
<type 'dict'>

In Pyston, the type of module's __dict__ is `attrwrapper:

>> print(type(__builtins__.__dict__))
<type 'attrwrapper'>

Pyston use the attrwrapper for performance reason. Can NumPy test accept that if run with Pyston?

Marius Wachtler
@undingen
May 11 2016 09:40
I think if numpy makes explicit comparisons of the type(v.__dict__) we should patch it for now to accept our attrwrapper.
which test triggers this?
Sun
@Daetalus
May 11 2016 09:54
core/tests/test_api.py, line 67
Marius Wachtler
@undingen
May 11 2016 10:03
we should just comment out the check
if the test passes without it
Sun
@Daetalus
May 11 2016 10:03
I see.
Marius Wachtler
@undingen
May 11 2016 10:29
@sun would do you think about updating the numpy version we test against to the last released one (1.11.0)?
Sun
@Daetalus
May 11 2016 10:39
Already updated it in my local host. But I am still working on some specific problem, just update the patch file, didn't get the fully updated result yet.
Marius Wachtler
@undingen
May 11 2016 10:45
cool, I also just checked except of that I could not run the test at first because it could not open the .egg file (not sure if this was because I did not start from a clean virtualenv) it gives me roughly the same number of errors :-).
we should also get some test which calls often between python and numpy to see how perf is...
Sun
@Daetalus
May 11 2016 15:31
I got an egg file error too, is it same as yours?
Failure: OSError (No such file /xxx/numpy_test_env_pyston_release/site-packages/numpy-1.11.0.dev0+51c96fd-py2.7-linux-x86_64.egg/numpy) ... ERROR
Marius Wachtler
@undingen
May 11 2016 15:46
yes it's the same... not sure what's causing it :-(.
as a temp fix I just unpacked the egg and renamed the directory to the egg file (including the .egg extension) :-D
Nathaniel J. Smith
@njsmith
May 11 2016 18:14
1) those test results are incredibly impressive! Though I wonder why when I run the numpy test suite locally I see an extra 800 tests. Maybe just version differences I guess...
2) I don't know what you're doing with eggs but it makes me nervous because eggs are very deprecated and certainly not tested with numpy; I'm actually somewhat surprised if they work at all.
3) I don't know any reason of the top of my head why numpy would care about the type of module.__dict__ but I'll take a look...
Marius Wachtler
@undingen
May 11 2016 18:54
the much lower number of tests is just because we currently test with some unknown older version.
can you recommend a benchmark which does stuff in python and numpy so that we can see how much the perf is changed with pyston / how much overhead we have when calling into numpy from python with pyston?
Nathaniel J. Smith
@njsmith
May 11 2016 19:29
There is a benchmark suite using the advice tool
*asv
But a simple starting point would be
benchmark 1: to check a mostly C operation, performance on CPython and Pyston should converage as problem size grows
```
arr = np.ones(N)
np.sum(arr)
benchmark 2: operation where C API overhead will dominate:
arr = np.ones(N)
total = 0
for i in range(N):
    total += arr[i]
Nathaniel J. Smith
@njsmith
May 11 2016 19:34
and you can run the asv benchmark suite with ./runtests.py --bench, or check out benchmarks/README.rst.
it's mostly microbenchmarks
Marius Wachtler
@undingen
May 11 2016 19:35
thanks, I will take a look!
Nathaniel J. Smith
@njsmith
May 11 2016 19:38
@Daetalus: I think that assert_(isinstance(builtins, dict)) isn't very important
IIRC there's a py2/py3 thing where on one of them __builtins__ is a module and on one of them it's a dict, so the line of code above is some tricky attempt to find the builtins dict regardless of what it starts with, and then the assert_ is just a check that the trickiness worked -- but if it didn't work we'd just crash later anyway
...actually, that doesn't make any sense, I just checked and __builtins__ is always a module on both py2 and py3, and __builtins.__dict__ always works. No idea why there is tricky code there to handle the case where not hasattr(__builtins__, "__dict__")
probably a PR to clean that test up would be accepted upstream
Nathaniel J. Smith
@njsmith
May 11 2016 19:45
....oh meh, no, it's messier than I thought. it's not a py2/py3 thing, it's that for some random reason, if you're in __main__ then __builtins__ is the builtins module, but if you're in any other namespace then __builtins__ is the dict of the builtins module
so you might have trouble with other code that checks whether isinstance(__builtins__, dict), given that there are two cases here that code has to be prepared to handle
that said I think we'd still accept a patch to clean that up :-)
Kevin Modzelewski
@kmod
May 11 2016 20:30
ok, with #1171 I think we have the cheetah test working again
interestly, we take a while to run it
cpython takes about 14 seconds, we take 34.
pypy takes 30, and we pass more tests than they do :)
Marius Wachtler
@undingen
May 11 2016 20:32
cool!, thanks for fixing it
Kevin Modzelewski
@kmod
May 11 2016 20:34
master runs the test in 31 seconds