These are chat archives for dropbox/pyston

14th
Apr 2015
Travis Hance
@tjhance
Apr 14 2015 00:13
that’s the primary lesson i’ve taken away from working on pyston
Chris Toshok
@toshok
Apr 14 2015 03:50
terrible hack gets django user list loading
hack = telling pyston that list.__new__ accepts varargs
Chris Toshok
@toshok
Apr 14 2015 14:55
hm, pypa seems to report syntax errors in methods against the first line of the method, not the line where the syntax error actually is
Marius Wachtler
@undingen
Apr 14 2015 16:22
can you give me an example
Chris Toshok
@toshok
Apr 14 2015 16:23
Yeah, on my way in the office. Will c&p when I arrive :)
Marius Wachtler
@undingen
Apr 14 2015 16:23
never mind, it's easy to reproduce
Chris Toshok
@toshok
Apr 14 2015 16:51
ok cool
Marius Wachtler
@undingen
Apr 14 2015 17:00
there is a strange? if vinzenz/libpypa#28 which let's it always prefer the ""parent" AST node with lower line number. Not sure about what's the best way to fix it.
swapping the if condition gave me much better error messages for my simple test but will probably report other errors which are currently correct at the wrong location?
because it will just about the last parsed token
Chris Toshok
@toshok
Apr 14 2015 17:09
yeah that looks like difficult logic to ensure it always works. is ast a fully parsed ast node at this point, and cur is the most recently lexed token? or is ast being constructed and cur is meant to be used to further construct ast?
Marius Wachtler
@undingen
Apr 14 2015 17:25
AFAIK: ast is the last fully parsed one
but I just looked 5 mins at the pypa source...
We should have found the issues a few months ago when vinzenz had more time to work on libpypa :-D
Chris Toshok
@toshok
Apr 14 2015 17:53
hm, actually my hack (changing list.__new__ to accept varargs) appears to be the way to fix this problem
cpython has list’s tp_new set to PyType_GenericNew, which is essentially our Box::operator new, only without the size parameter
Chris Toshok
@toshok
Apr 14 2015 18:32
awesome, can get to the user/group property UI’s now in django/admin
and they’re sloooow
52% of perf time over 1000 requests in PythonFrameIterator::incr + _Unwind_Find_FDE + _ULx86_64_Ifind_dynamic_proc_info
87% of the first one is from getTraceback(), so definitely glad that’s getting looked at :)
Marius Wachtler
@undingen
Apr 14 2015 18:36
:-(
but maybe it's not to bad if it's mostly a single thing which slows down everything
Chris Toshok
@toshok
Apr 14 2015 18:36
if anything, just points out that unwinding is the right place to be putting energy :)
yup
after those top three, the percent runtime of the next hottest function drops to 1.72%
Marius Wachtler
@undingen
Apr 14 2015 18:37
:-)
s* I installed the latest updates on my ubuntu notebook which made the boot partition got completely full. and now it doesn't start up completely :-(
Chris Toshok
@toshok
Apr 14 2015 18:41
ugh
so the last thing keeping django user/group properties pages from working was that we define str.__iter__ and cpython doesn't
django checks if __iter__ is available, and returns obj if it is, [obj] if it isn't
so we were iterating over the characters in a string
Marius Wachtler
@undingen
Apr 14 2015 18:51
wow that's hard to track down
Chris Toshok
@toshok
Apr 14 2015 19:26
odd. removing __iter__ from str_cls causes an exception when run in the jit
Marius Wachtler
@undingen
Apr 14 2015 21:19
@toshok I'm interested to test the performance of the slow django part with my stack unwinding patch applied. Do you have a repo with the changes I need for django available?
Chris Toshok
@toshok
Apr 14 2015 21:23
yeah this should have everything you need to login: https://github.com/toshok/pyston/tree/django-fixes
requires the same instructions as from kmod’s django branch email
Marius Wachtler
@undingen
Apr 14 2015 21:23
ok thanks
what page do I have to access?
Chris Toshok
@toshok
Apr 14 2015 21:24
i want to say anything post-login is slow, so you could try like: http://localhost:8000/admin/auth/group
once you load that url in a browser, grab the csrftoken and sessionid cookies
then you can do ab -C “csrftoken=$csrf_token_from_browser” -C “sessionid=$session_id_from_browser” -n 1000 $url
Marius Wachtler
@undingen
Apr 14 2015 21:26
ok thanks will try
Chris Toshok
@toshok
Apr 14 2015 21:30
okay, think i’ve narrowed it down. UnknownType::getPystonIter generates code that calls callattr(‘__iter__’), then checks if the return value is null. if it is it falls back to creating an iterhelper
trouble is that the call to callattr throws an exception instead of returning null
this also explains the difference in exceptions from this:
>>> for i in 10: print i
Marius Wachtler
@undingen
Apr 14 2015 21:32
then it was me who introduced the bug..
Chris Toshok
@toshok
Apr 14 2015 21:32
for python: TypeError: 'int' object is not iterable
for pyston: AttributeError: 'int' object has no attribute ‘__iter__’
ahah, yeah rewriting for GET_ITER
Marius Wachtler
@undingen
Apr 14 2015 21:34
I'm wondering why it throws because I'm setting: .null_on_nonexistent = true
in compvars.cpp:358 or are we talking about different things?
Chris Toshok
@toshok
Apr 14 2015 21:34
it’s the call to var->callattr instead of tryCallattrConstant
yeah I think callattr isn’t respecting null_on_nonexistent
think i can fix this. just check flags.null_on_nonexistent, and if it’s true, pass a valid pointer to tryCallattrConstant’s no_attribute
lack of that pointer causes it to embed llvm IR to unconditionally throw the exception
Marius Wachtler
@undingen
Apr 14 2015 21:39
ah your right, I missed that the flag get's ignored in some places :-(
Marius Wachtler
@undingen
Apr 14 2015 22:39
ok before the FP stack unwinding: mean response time 940ms after: 740ms. hoped for more
Marius Wachtler
@undingen
Apr 14 2015 22:45
_Unwind_Find_FDE keeps showing up with ~10% when running perf top
Michael Arntzenius
@rntz
Apr 14 2015 22:45
@undingen by "FP stack unwinding", you mean the PR you issued which changes getTraceback() not to use libunwind?
Marius Wachtler
@undingen
Apr 14 2015 22:47
yes but I changed it to also use the FP stack unwinding when ever possible e.b. for getting the top stack frame when calling getGlobals() etc
Marius Wachtler
@undingen
Apr 14 2015 22:56
I updated the pull request to the current code I'm testing with. (But one has in addition to compile pypa with frame pointers in order to not segfault on our testsuite)
Michael Arntzenius
@rntz
Apr 14 2015 22:57
@toshok When you did the 1000-request test on django, was that with the -I flag or not?
@undingen and when you say _Unwind_Find_FDE keeps showing up, also, is that with -I flag or not?
Chris Toshok
@toshok
Apr 14 2015 22:57
without -I
Marius Wachtler
@undingen
Apr 14 2015 22:58
no without the -I flag just with standard options except of disabling pypa
Michael Arntzenius
@rntz
Apr 14 2015 22:58
ok.
Marius Wachtler
@undingen
Apr 14 2015 23:04
when I add -I the _Unwind_Find_FDE disappears from the top as expected
Marius Wachtler
@undingen
Apr 14 2015 23:09
nobody will believe that we were there: http://lwn.net/images/2015/pls-group.jpg aka where is waldo?
Chris Toshok
@toshok
Apr 14 2015 23:10
haha, our heads are strategically hidden
behind one of the pypy guys, in my case
Kevin Modzelewski
@kmod
Apr 14 2015 23:44
@undingen how close is the FP-based unwinding to working?
we're thinking of using it for some _getframe hacks :P
I guess we could do this with libunwind as well
Marius Wachtler
@undingen
Apr 14 2015 23:46
should work. Should only need small cleanup to reduce number of places where I do the current_frame increment to reduce duplicate code. And may need makefile or cmake change if it crashes when parsing a unicode string with pypa and an exception gehts thrown.