These are chat archives for dropbox/pyston

12th
May 2015
synecdoche @synecdoche heads over to dropbox for dinner
Kevin Modzelewski
@kmod
May 12 2015 03:14
@undingen @toshok what do you guys think about #515?
(a formatting change)
Chris Toshok
@toshok
May 12 2015 03:16
definitely helps, imo
i almost never scroll to find code, so the increase in line count won’t bother me
Chris Toshok
@toshok
May 12 2015 03:32
hm, calling class methods might be more expensive than our slow paths :/
at least for __hash__ and __eq__ (for dicts)
since they go through lookup_maybe which boxes a string (i’ve got a patch to fix this someplace), allocate a bound function, and allocate an arg tuple
Marius Wachtler
@undingen
May 12 2015 06:32
#515 looks easier to read and should make diffs easier to read but I don't care to much about it I have line wrapping enablen which is also okay.
Travis Hance
@tjhance
May 12 2015 15:05
Also, we could really use a way to do dictionary lookups without having to box a string
Chris Toshok
@toshok
May 12 2015 15:06
we can probably do something like that by using an unordered_map<llvm::StringRef, Box*> in BoxedDict
or a StringMap i suppose
problem is we need to maintain strong refs to the keys, so we’d need another std::vector<Box*, StlCompatAllocator<Box*>> that stores everything used as a key, or something
Travis Hance
@tjhance
May 12 2015 15:10
Or we just need a way to lookup a llvm::stringref in a unordered_map <Box*, Box*>
Chris Toshok
@toshok
May 12 2015 15:11
yeah. we could maybe create a BoxedString on the stack and use that?
Travis Hance
@tjhance
May 12 2015 15:11
Although I don't know if stl gives us this capability? But in theory it should be possible
Ooh yeah we could do that
Chris Toshok
@toshok
May 12 2015 15:11
a tagged union for the key? unordered_map<either<Box*,llvm::StringRref, Box*> ?
Marius Wachtler
@undingen
May 12 2015 15:12
or we create a special dict when all keys are strings (and not sublasses of string) and intern all string (like cpython does imho) then it should only be a llvm::DenseMap<BoxeedString*,...>
Chris Toshok
@toshok
May 12 2015 15:12
that wouldn’t work i suppose
Marius Wachtler
@undingen
May 12 2015 15:13
than lookups should be super cheap if they are only a pointer comparison
Chris Toshok
@toshok
May 12 2015 15:13
anything that gets us closer to not needing StlCompatAllocator in the unordered_map would make me happy :)
i’ve removed it locally for some microbenchmark stuff, replacing it with an explicit walk over the unordered_map inside dictGCHandler
Marius Wachtler
@undingen
May 12 2015 15:15
I'm wondering if using a llvm::DenseMap woould improve the performance
Chris Toshok
@toshok
May 12 2015 15:15
what are the characteristics of ::DenseMap? no buckets?
oh man, that sounds perfect
Marius Wachtler
@undingen
May 12 2015 15:16
yeah I think all the key values pairs are allocated in a single allocation instead of one per pair
Chris Toshok
@toshok
May 12 2015 15:16
DenseMap is a simple quadratically probed hash table. It excels at supporting small keys and values: it uses a single allocation to hold all of the pairs that are currently inserted in the map. DenseMap is a great way to map pointers to pointers, or map other small types to each other
that’s exactly what we need to fix wrt unordered_map using conservative allocations
Marius Wachtler
@undingen
May 12 2015 15:18
but it looks like it doesn't support custom alloc routines
Chris Toshok
@toshok
May 12 2015 15:19
that should be fine if we can at least get access to its backing store
Marius Wachtler
@undingen
May 12 2015 15:19

BucketT *getBuckets() const {
return Buckets;
}

unsigned getNumBuckets() const {
return NumBuckets;
}

Chris Toshok
@toshok
May 12 2015 15:19
getBuckets() is private, boo
:(
Marius Wachtler
@undingen
May 12 2015 15:19
oh :-(
Chris Toshok
@toshok
May 12 2015 15:20
#define private public #include <llvm/ADT/DenseMap.h #undef private :)
err
Travis Hance
@tjhance
May 12 2015 15:21
Cpython used quadratic probing as well, iirc
We can always just, like, pull the source file into our project and modify it
Chris Toshok
@toshok
May 12 2015 15:22
yes
Marius Wachtler
@undingen
May 12 2015 15:22
yes or we could create our own class derived form DenseMapBase one basically just copying the existing DenseMap class because it looks like it's small and only contained in the header and make it public in our implementation
Travis Hance
@tjhance
May 12 2015 15:22
We do that a lot:P
Chris Toshok
@toshok
May 12 2015 15:22
hehe
Marius Wachtler
@undingen
May 12 2015 15:23
Then we could supply our own malloc routines
Chris Toshok
@toshok
May 12 2015 15:25
i have a dict getitem microbenchmark that takes 2.1s on cpython and over a minute on pyston. using a runtime IC for __hash__ drops it to 38 seconds, and removing StlCompatAllocator drops it to 5. so something DenseMap-like/derived would definitely be a good thing if we can get the GC stuff to work out. DenseMap would likely be faster still, since perf shows most time being spent in the mark phase still (due to the dictionary iteration, I expect)
i’m wary of using the gc for lots of small conservative allocations
but DenseMap won’t be as bad as unordered_map
Marius Wachtler
@undingen
May 12 2015 15:27
:-)
Marius Wachtler
@undingen
May 12 2015 15:53
Today I played a little bit around with the django-template benchmark. Because I wanted to know what the test does and have at least a slight idea how the perf profile looks like.
While trying different stuff I modified the django source slightly with the result that pypy got 2x slower than cpython
:-D
Chris Toshok
@toshok
May 12 2015 16:04
Wow
Marius Wachtler
@undingen
May 12 2015 16:19
using the cpython profiler I noticed that Node::get_nodes_by_type is one of the hottest functions but at the same time it's one which pyston constantly rewrites, because of the different Node subclasses which it get called on. So I added the method implementation to the most frequent sublasses. and I did something similar to the TokenParser. With the result that the pyston performance only very slightly improved but the pypy performance regressed :-D
Chris Toshok
@toshok
May 12 2015 17:57
We should give up rewriting it after a point right? The megamorphic threshold?
toshok @toshok waits to have tooth drilled :(
Chris Toshok
@toshok
May 12 2015 18:00
If you can push a branch with that change I'll give it a try on the django fixes branch. The differences in time at HEAD there are pretty small so the rewrite change might make a big difference
Rudi Chen
@rudi-c
May 12 2015 18:01
If we get an assertion in C++, is there a good way to know what python code it's currently trying to run? My crash happens in the interpreter when pyston loads.
Chris Toshok
@toshok
May 12 2015 18:02
You should be able to get the sourceinfo from the ast_interpreter frames
Marius Wachtler
@undingen
May 12 2015 18:25
yes it constantly hits the megamorphic threshold - it will abort rewriting after 100 times. But because cpythons profiling extension says this is one of the hottest functions I thought it may be good to not abort rewriting on those...
Chris Toshok
@toshok
May 12 2015 18:26
Ah I see. And yeah, it's definitely the hot spot for parsing (and all the sub-parsing) django does
Marius Wachtler
@undingen
May 12 2015 18:37
@toshok dentist.. :-(
Chris Toshok
@toshok
May 12 2015 19:38
All done at least. Going to be sore in a few hours :)
Chris Toshok
@toshok
May 12 2015 21:58
uh… huh.
$ /bin/time ./pyston_release -x -s -T minibenchmarks/django-template.py
1.70user 0.30system 0:02.01elapsed 99%CPU (0avgtext+0avgdata 146212maxresident)k
0inputs+144outputs (0major+97688minor)pagefaults 0swaps

toshok@jitdev:~/pyston$ PYTHONPATH=`pwd`/lib_pyston time python minibenchmarks/django-template.py
1.71user 0.05system 0:01.77elapsed 99%CPU (0avgtext+0avgdata 24752maxresident)k
0inputs+5456outputs (0major+10033minor)pagefaults 0swaps
that’s with timers enabled too, with the associated (non-zero) runtime cost
Marius Wachtler
@undingen
May 12 2015 22:02
congratulations, what did you change? or what did improve it the most?
Chris Toshok
@toshok
May 12 2015 22:02
ahh, I was tricked. earlier in the output: ValueError: could not convert string to float: model.admin_url
along with sys.excepthook is missing
must be rebase fallout
Marius Wachtler
@undingen
May 12 2015 22:03
:-(, that would have really surprised my. If you already got such a large speedup
Chris Toshok
@toshok
May 12 2015 22:03
yeah :(
Marius Wachtler
@undingen
May 12 2015 22:04
I'm also getting the sys.excepthook thing lately when an exceptions gets printed, not sure why but I think we can ignore it (or add it :-D) because the default implementation of sys.excepthook does the same as the else branch when it can't find it.
ohoh getting this the warning the first time from gmail on a github email:
Be careful with this message. It contains content that's typically used to steal personal information. Learn more
Report this suspicious message   Ignore, I trust this message
Merged #520.

—
Reply to this email directly or view it on GitHub.
Since when do phishing emails look like a merged pull request? :-D
Kevin Modzelewski
@kmod
May 12 2015 22:08
lol that's a very specific kind of phishing
very well-tailored
Marius Wachtler
@undingen
May 12 2015 22:08
:+1:
Chris Toshok
@toshok
May 12 2015 22:08
wow
found in the gdb manual: This is currently only available for HP-UX.
Marius Wachtler
@undingen
May 12 2015 22:12
HP-UX Latest release: 11i v3 Update 14 / March 2015; 2 months ago :-)
Chris Toshok
@toshok
May 12 2015 22:12
wow
that is amazing
Marius Wachtler
@undingen
May 12 2015 22:13
New features/products - Java 8.0.00 and 10GigEthr-05
Chris Toshok
@toshok
May 12 2015 22:13
HP constantly gave hardware grants to my uni. I still have hpux flashbacks :)
Marius Wachtler
@undingen
May 12 2015 22:13
but if the wikipedia articles is correct than the update one before was 2007
:-). But we have a IBM AIX ifdef inside thread_pthread.h :-P
Chris Toshok
@toshok
May 12 2015 22:15
my only aix memory is their package manager/sysadmin console
i think it was called “smit”?
it had an animation of a man running whenever it was performing a task. if the task failed/errored, the man would fall down
successful man
Marius Wachtler
@undingen
May 12 2015 22:19
lol much better than the windows folder / flying document animation
Chris Toshok
@toshok
May 12 2015 22:31
alas, it appears the ValueError: could not convert string to float: model.admin_url is due to USE_CMAKE=0
the cmake build doesn’t fail in that way
which makes me think the libgcc patch has a bug in it
or possibly some other cmake dep is newer than non-cmake
Marius Wachtler
@undingen
May 12 2015 22:38
could it be becuase of my libpypa upgrade?
but it would be strange because I would expect that the make one would work (old version) and the new one (cmake) would fail
if I introduced a new bug
Chris Toshok
@toshok
May 12 2015 22:51
i made sure to upgrade my pyston_deps/pypa
i think it’s okay. hopefully the timers will show exception handling + something else :)