These are chat archives for dropbox/pyston

29th
Jul 2015
Sun
@Daetalus
Jul 29 2015 07:27

Hi, I am working on old style class support see #254. Most of these were accomplished except few.

  • __getslice__, __setslice__, __delslice__ is deprecated since Python 2.0. CPython will call __getitem__ and some correspond functions, never call it. But CPython still implement it. Should we support these three functions?
  • __get__, set,delete` only work if the descriptor is NEW style class. And the Pyston behaviors of these three functions are exact same as CPython and PyPy.

Details:
Descriptor(Old), Owner(Old) - won't call either __get__, __set__, or __del__.
Descriptor(New), Owner(Old) - only call __get__.
Descriptor(New), Owner(New) - all of three functions could work.

  • __coerce__, Pyston seems doesn't implement this.

Any suggestion?

Rudi Chen
@rudi-c
Jul 29 2015 07:32
Oh shoot, I forgot about old style classes when I implemented all the missing slicing edge cases.
As much as possible try to aim to match CPython's behavior.
It has a lot of semantics about when to fall back from getslice to getitem and things like that.
See the test slice.py
Kevin Modzelewski
@kmod
Jul 29 2015 07:36
yeah I agree, I'd say just try to do it however cpython does
(though I don't know how that is for these cases)
Sun
@Daetalus
Jul 29 2015 07:44

Ok, I will add __getslice__, __setslice__, __delslice__. And also add tests into slice.py test.

And I think __get__, __set__, __delete__ no need further work for old style class. It match CPython's behavior.

coerce considered as "Non-essential Built-in Functions", should I implement it as a builtin function in Pyston?

Rudi Chen
@rudi-c
Jul 29 2015 07:48
There's no harm in supporting it.
Speaking of coercion, there's a few coercion behavior that we don't support at the moment if you're interested. Like Int + Complex or Float + Complex
Sun
@Daetalus
Jul 29 2015 07:50
Ok, I will investigate it.
Rudi Chen
@rudi-c
Jul 29 2015 07:50
:)
Sun
@Daetalus
Jul 29 2015 17:40
Hello?
Rudi Chen
@rudi-c
Jul 29 2015 17:43
'morning
Sun
@Daetalus
Jul 29 2015 18:24

Hi,
I am try to implement __setslice__ for old style class (__getslice__and __delslice__ were already done). But get stucked. Here is the function signature:

object.__setslice__(self, i, j, sequence)

In Python it will called like this:

f[x, y] = [foo, bar] #f is a instance of old style class

I imitate the instance_ass_slice CPython logic and Pyston exsit instance function implementation. But it will report Assertiongc::isValidGCObject(args[i - 3]) || args[i - 3] == __null' failed.inobjmodel.cpp:3592`.

Here is my implementation and backtrace imformation:
https://gist.github.com/Daetalus/2f9da47f5e0dc247b20e

I did some investigation, but couldn't solve it. Could anyone can give any helps?

Sorry for the blank lines...
Assertiongc::isValidGCObject(args[i - 3]) || args[i - 3] == __null' failed.inobjmodel.cpp:3592`
Rudi Chen
@rudi-c
Jul 29 2015 18:26
Do you know which half of the assertion fails?
Sun
@Daetalus
Jul 29 2015 18:27
First half, I verified it by gdb
Rudi Chen
@rudi-c
Jul 29 2015 18:30
I think the runtime call needs to be 4 arguments (it needs the self argument).
Actually I'm not sure if that's right.
Sun
@Daetalus
Jul 29 2015 18:36
Ok, I need to go to bed. Hope I could find a solution at tomorrow.
Rudi Chen
@rudi-c
Jul 29 2015 18:36
Can you make a patch with your changes?
I could try to run it.
Sun
@Daetalus
Jul 29 2015 18:36
Thanks, 陈!
Ok, I will try.
Sun
@Daetalus
Jul 29 2015 18:49
@rudi-c How to send you the patch?
Rudi Chen
@rudi-c
Jul 29 2015 18:49
A gist should be fine
Alternatively, a PR
Sun
@Daetalus
Jul 29 2015 18:57
Please see here.
https://gist.github.com/Daetalus/10eb26b9c72e588f040b
The patch was diff from current dropbox/master branch.
Rudi Chen
@rudi-c
Jul 29 2015 18:58
Ok great. I'll take a look sometime later today.
Thanks :)
Sun
@Daetalus
Jul 29 2015 18:58
Thanks. And I hope I could make some contribution to NumPy support in next week. This week is too busy...
Marius Wachtler
@undingen
Jul 29 2015 19:00
@Daetalus I thin kthe problem is that instanceSetitem(inst, slice, sequence);should be instanceSetitem(inst, slice, *sequence);
We pass the first 3 args as separate args but addition args are passed in an array... so the 4th arg is actual a pointer to an array of the additional args in your case only one
Sun
@Daetalus
Jul 29 2015 19:05
Thanks @undingen , I will check it.
Sun
@Daetalus
Jul 29 2015 19:12
Ah!!!!!!!!!!!
Solved!!!!!!!!!!!!!!!
Thanks! @undingen , @rudi-c I have to sleep... I will add some tests tomorrow and create a PR.
Marius Wachtler
@undingen
Jul 29 2015 19:15
:-)
Marius Wachtler
@undingen
Jul 29 2015 20:28

@Daetalus to make it more obvious what's going on you could use:

Box* instanceSetslice(Box* _inst, Box* i, Box* j, Box** args) {
    Box* sequence = args[0];
    [...]

and then you can use sequence normally without having to dereference it first.

Kevin Modzelewski
@kmod
Jul 29 2015 20:40
looks like most of the django_template3 improvements came from bjit improvements :)
Rudi Chen
@rudi-c
Jul 29 2015 20:42
bjit?
Kevin Modzelewski
@kmod
Jul 29 2015 20:43
baseline jit
:-)
Rudi Chen
@rudi-c
Jul 29 2015 20:47
Wow there were some massive drops there.
Marius Wachtler
@undingen
Jul 29 2015 20:49
but it's still takes 2x as long as with enabled llvm tier. Will look into that this benchmark may tell more why it's still slower (normally it's not as much)
Travis Hance
@tjhance
Jul 29 2015 20:50
where is cpython on that graph
Marius Wachtler
@undingen
Jul 29 2015 21:01

concerning the cheetah problem:
the problem I'm seeing can be reduced to:

class C(object):
    pass
print C.__str__ is object.__str__

CPython will print True while we print False.
I think the difference is that the object.__str__ inside cpython is implementation as tp_str and we create one of our BoxedFunctions and assign it the __str__ attribute.
This means that the type(C.__str) is a wrapper_descriptor and for us it's a temporary instance method. Because in descriptorClsSpecialCases (called from getattr) we call return boxUnboundInstanceMethod(descr, cls);.
I don't know how to fix this. (Maybe by switching to tp_attr?)

And I'm looking for help
Kevin Modzelewski
@kmod
Jul 29 2015 21:03
oh erf
yeah easiest is probably to switch to tp_str
the only reason not to would be if there's a perf hit from doing that, but I don't think there will be
(ie our python functions are sometimes more rewriteable than the tp slots, but I don't think there are many direct calls to `_str`)
also, pypy says False as well
not that that really helps us if cheetah relies on this behavior
Marius Wachtler
@undingen
Jul 29 2015 21:10
yes they explicitly tell at https://bitbucket.org/pypy/compatibility/wiki/cheetah that one has to patch cheetah. But I thought it would be nice if we support it without patching
Rudi Chen
@rudi-c
Jul 29 2015 23:59
Has anyone seen the GC crash lately?