These are chat archives for dropbox/pyston

24th
Jul 2015
Chris Toshok
@toshok
Jul 24 2015 00:00
oh nevermind, i see the problem :)
Travis Hance
@tjhance
Jul 24 2015 01:52
hii
you said dropbox/pyston#751 should fix the max_align_t issue, right?
Chris Toshok
@toshok
Jul 24 2015 01:52
yeah
Travis Hance
@tjhance
Jul 24 2015 01:52
I’m up-to-date on master but still getting it:\
Chris Toshok
@toshok
Jul 24 2015 01:52
it does… doesn’t it? :(
hrm
Travis Hance
@tjhance
Jul 24 2015 01:52
do I need to nuke and rebuild again?
Chris Toshok
@toshok
Jul 24 2015 01:53
where’s the error happen?
libpypa or src/runtime?
probably need to git submodule update if it’s libpypa
Travis Hance
@tjhance
Jul 24 2015 01:53
ooh i ndeed to git submodule update
Chris Toshok
@toshok
Jul 24 2015 01:53
phew
Travis Hance
@tjhance
Jul 24 2015 01:53
f u submodules
Travis Hance
@tjhance
Jul 24 2015 02:20
                           5071e380a747b77ba4:  9fa348a911fc8c3c12:
      django_template2.py             4.4s (6)             0.3s (6)  -92.8%
            pyxl_bench.py             3.2s (6)             0.2s (6)  -94.0%
sqlalchemy_imperative2.py             3.5s (6)             0.3s (6)  -92.8%
                  geomean                 3.7s                 0.2s  -93.2%
time for that 0.4 release? :)
Chris Toshok
@toshok
Jul 24 2015 02:21
haha
oh man
Sun
@Daetalus
Jul 24 2015 02:21
Oh...Impressive!
Chris Toshok
@toshok
Jul 24 2015 02:21
is there an easy way to bulk remove builds/runs?
Travis Hance
@tjhance
Jul 24 2015 02:21
remove the data file?
analysis/data.db
Chris Toshok
@toshok
Jul 24 2015 02:22
but won’t that clobber everything?
Travis Hance
@tjhance
Jul 24 2015 02:22
yeah
idk, do you care?
Chris Toshok
@toshok
Jul 24 2015 02:22
yeah, i was just going to say there aren’t often times when I do
and usually it’s just 2-3 hashes and a couple of builds each
Travis Hance
@tjhance
Jul 24 2015 02:25
yeah I basically consider old times to be useless
like no sense in comparing times from yesterday to times from today
Chris Toshok
@toshok
Jul 24 2015 02:27
well, and you can regenerate them if you need to
Travis Hance
@tjhance
Jul 24 2015 02:27
right
Chris Toshok
@toshok
Jul 24 2015 02:41
-I/mnt/toshok/pyston-install/include/pyston2.7
well there’s yer problem
Sun
@Daetalus
Jul 24 2015 04:24
@kmod #739 updated.
Marius Wachtler
@undingen
Jul 24 2015 08:47
Did anyone else experience a crash inside pyston::gc::hasOrderedFinalizer while running the sqlalchemy_smalltest.py?
Marius Wachtler
@undingen
Jul 24 2015 09:05
Oh could be a problem which I introduces with my change
Chris Toshok
@toshok
Jul 24 2015 15:15
@kmod and I have seen that assert
err, a crash inside? no.. an assert though, yes
Marius Wachtler
@undingen
Jul 24 2015 15:24
yes it's an assert, in the meantime I also figured out that you have seen it and I didn't introduce it
Marius Wachtler
@undingen
Jul 24 2015 15:37
@kmod when you are reviewing my changes could you please start with #757 before #736
Rudi Chen
@rudi-c
Jul 24 2015 17:38
@undingen Still trying to repro that one :(
Marius Wachtler
@undingen
Jul 24 2015 17:41
:-( is this only producible on travis-ci?
Rudi Chen
@rudi-c
Jul 24 2015 17:42
It seems to be very random, both on travis and locally.
I think kmod got it locally at some point.
Marius Wachtler
@undingen
Jul 24 2015 17:55
:-(
Rudi Chen
@rudi-c
Jul 24 2015 18:49
Is there any circumstances where the attrs_offset of BoxedClass would have a value different than 0 or offsetof(BoxedClass, attrs)?
Travis Hance
@tjhance
Jul 24 2015 18:50
attrs_offset is the offset of the attrs in an object that the BoxedClass is a class of
Rudi Chen
@rudi-c
Jul 24 2015 18:51
Oh right. offsetof(BoxedClass, attrs) is only for metaclasses
read the rules on tp_dictoffset; attrs_offset follows the same rules
Sun
@Daetalus
Jul 24 2015 19:41

Hi, I am working on old style operator support. Seems there has inconsistent error message with __pow__ in Python. Please correct me if I am wrong.

Here is the code and with descriptions:
https://gist.github.com/Daetalus/ec5034f08f597fee00de

Sorry, I add a lot of blank lines?
Kevin Modzelewski
@kmod
Jul 24 2015 19:48
sorry, are those the messages that cpython outputs?
@tjhance I thought of a potential project -- it'd be nice if we could have some ability to guard in the middle of a patchpoint (ie after mutations)
ex for the callattr-of-descriptor case, but also for binops and calling constructors
I was thinking maybe one simple approach would be to say
"until the next mutation occurs, the slowpath is now [this other function call]"
Sun
@Daetalus
Jul 24 2015 20:04
Yes, CPython output
Kevin Modzelewski
@kmod
Jul 24 2015 20:04
hmm interesting
I'd say that it's not super important to match the error messages exactly
Sun
@Daetalus
Jul 24 2015 20:04
Ok
Kevin Modzelewski
@kmod
Jul 24 2015 20:04
we should make sure to match the exception types though
you can check out CPython's instance_pow to see a bit about where the different error messages come from
there are two completely different codepaths for instance_pow if the mod was passed or not
Sun
@Daetalus
Jul 24 2015 20:07
Ok, I will dig that. But should the exception be same? No matter pass mod or not.
Rudi Chen
@rudi-c
Jul 24 2015 20:08
How do you run the tests in /test/test_extension/?
Travis Hance
@tjhance
Jul 24 2015 20:09
IIRC those are just files imported by tests in /test/tests/
Kevin Modzelewski
@kmod
Jul 24 2015 20:09
sorry, do you mean should the exceptions be the same as cpython's, or should they be the same as each other on the two branches?
one interesting thing to try is to run that test under pypy and see what error messages they give
Rudi Chen
@rudi-c
Jul 24 2015 20:09
@tjhance Ah looks like it. Thanks!
Sun
@Daetalus
Jul 24 2015 20:11
"should they be the same as each other on the two branches" , testing in PyPy
Kevin Modzelewski
@kmod
Jul 24 2015 20:12
well if CPython doesn't do it then we probably don't :)
Sun
@Daetalus
Jul 24 2015 20:13
PyPy give same type of Error.
<type 'exceptions.TypeError'> operands do not support **
<type 'exceptions.TypeError'> operands do not support *
Miss the last * in second line.
Kevin Modzelewski
@kmod
Jul 24 2015 20:15
sounds like it'd be fine to go either way
I'd say, let's focus on the behavior and not worry too much about the exact error messages
Sun
@Daetalus
Jul 24 2015 20:16
So don't check the error message in test?
Kevin Modzelewski
@kmod
Jul 24 2015 20:17
yeah I think that's fine
you can just do something like
try:
  print pow(a, b, c)
except Exception as e:
  print "threw a", type(e)
Sun
@Daetalus
Jul 24 2015 20:17
Got it, thanks!
But, type(e) are the same. One is attribute error, the other is type error
Kevin Modzelewski
@kmod
Jul 24 2015 20:19
oh interesting
normally I would say it's important that we match the error types that cpython generates
but if pypy gets away with different ones, then it's probably ok to have them be different
you can just do something like assert type(e) in (AttributeError, TypeError)
Sun
@Daetalus
Jul 24 2015 20:21
Got it.
Kevin Modzelewski
@kmod
Jul 24 2015 20:21
with maybe a note about how the exceptions can be different
Sun
@Daetalus
Jul 24 2015 20:29

Another questions, about __oct__, (the code below as reminder)

        class Foobar(long):
            def __oct__(self):
                # Returning a non-string should not blow up.
                return self + 1

        test_exc('%o', Foobar(), TypeError,
                 "expected string or Unicode object, long found")

I did some research today. But still don't know how to check the custom __oct__ return type. longOct return string directly.
nb_oct seems abandoned in CPython.

Kevin Modzelewski
@kmod
Jul 24 2015 20:31
well it's ok to call Foobar().__oct__()
or long.__oct__(FooBar())
so the different return type isn't necessarily an error
except when it's used in "%o" formatting or in the oct() function
the place that it should get checked is _PyString_FormatLong
we already have it actually :) it's
    buf = PyString_AsString(result);
    if (!buf) {
        Py_DECREF(result);
        return NULL;
    }
the problem is more that we're not calling the appropriate subclass __oct__ if one was defined
Rudi Chen
@rudi-c
Jul 24 2015 20:36
Wow a segfault this time.
Looks like it's a memory problem (as opposed to a class having both a simple destructor and a del)
Kevin Modzelewski
@kmod
Jul 24 2015 20:38
yeah maybe the class got freed or something
Rudi Chen
@rudi-c
Jul 24 2015 20:38
Again :(
Sun
@Daetalus
Jul 24 2015 20:40
So do you mind to give some hints about how to call the appropriate subclass __oct__ and check the return type?
Kevin Modzelewski
@kmod
Jul 24 2015 20:41
I think it should just work to undo the "Pyston changes" in that function
ie remove the assert that it's a long (and not a subclass of long), and instead of calling the long functions (ex longOct) directly, call nb_oct
Sun
@Daetalus
Jul 24 2015 20:43
Ah, I already remove the assert. but didn't noticed the nb_oct.
Sorry for didn't find the obviously problem.
Sun
@Daetalus
Jul 24 2015 21:29
@kmod #739 updated
Kevin Modzelewski
@kmod
Jul 24 2015 21:39
merged :)
Rudi Chen
@rudi-c
Jul 24 2015 21:40
:+1:
Sun
@Daetalus
Jul 24 2015 21:41
Thanks for your patiently review.
Kevin Modzelewski
@kmod
Jul 24 2015 22:32
oh man
so django doesn't have any caching of parent templates
at least, by default
so when we render the admin page, it goes to disk to fetch admin/base_site.html and admin/base.html and then parses them
Kevin Modzelewski
@kmod
Jul 24 2015 22:41
and now we have django_template3
Marius Wachtler
@undingen
Jul 24 2015 23:05
concerning django_template and django_template2, does 2 supersede the first or do they test different stuff?
and should I use both (/or all three :-) from now on)
?
Kevin Modzelewski
@kmod
Jul 24 2015 23:09
well, they're getting progressively more rendering-heavy instead of parsing-heavy
and I think that that's more what we're interested in testing
but maybe we should keep the original django_template around to test parsing speed as well
since it's a deliberate design decision on their part to not cache the templates (so that you see any changes immediately), so it might reflect some workloads
so anyway, I think I'm going to start focusing on django_template3 over the earlier ones
since it's more in line with what pypy is testing with their django test
and when we talk about it we can say "it's like pypys bm_django but better" and we don't have to get into arguments about whether we should be testing parsing vs rendering
Marius Wachtler
@undingen
Jul 24 2015 23:24
k
and btw could we please get more frequent speed.pyston.org updates...
Kevin Modzelewski
@kmod
Jul 24 2015 23:26
ok :)
Marius Wachtler
@undingen
Jul 24 2015 23:27
nice for checking if non-perf changes don't introduce a perf regression
Travis Hance
@tjhance
Jul 24 2015 23:43
can we just keep all 3 in investigate.py?
maybe that would take too long to run
but i feel like more data points is better
like
if you make a change see
+1%, 0%, and -1%
it’s like, well this might have a kinda big effect, but I have no idea if it’s a good effect more often than it is a bad effect
Kevin Modzelewski
@kmod
Jul 24 2015 23:49
yeah I agree
I didn't want to have them all in the geomean though since it'll distort it to being django-only
Travis Hance
@tjhance
Jul 24 2015 23:49
hm yeah
okay we need more diverse benchmarks:)
Kevin Modzelewski
@kmod
Jul 24 2015 23:50
if someone wants to add in the "unaveraged" thing feel free :)
Travis Hance
@tjhance
Jul 24 2015 23:50
i mean it probably wouldn’t help anyway to have benchmarks that are too similar
hmm