These are chat archives for dropbox/pyston

14th
May 2015
Michael Arntzenius
@rntz
May 14 2015 00:11
okay, this is really weird. I think I've found a bug in Python (both 2 and 3) by modifying one of our internal tests (thread_forking.py), which we seem to be failing on the unwinder :( (but only in release mode, oddly!).
however, the bug is a deadlock (or occasionally an internal Python assert), so maybe my code is just using python's threading library incorrectly somehow
does anyone see any problems with this code: https://gist.github.com/rntz/36a2fb668a5d2bbe5b92
Chris Toshok
@toshok
May 14 2015 00:13
so given all the things that will make generalizing IC’s hard (looking at you, mro), would it make sense instead to build up PICs?
so instead of a IC slot mapping to if (the guards match) { do something } else slowpath;, it would map to if (the guards for the first rewrite match) { do first something } else if (the guards for the second rewrite match) { do second something } else …. else slowpath;
Chris Toshok
@toshok
May 14 2015 00:18
that wouldn’t fix the TranslateParser problem (where the subclass is declared inside a closure) directly, but it would fix other cases of higher ordered functions
Kevin Modzelewski
@kmod
May 14 2015 00:28
@toshok I'm not quite sure how what you're describing is different from what we have
I mean, our support is pretty rudimentary because we statically determine the number of polymorphic slots
Chris Toshok
@toshok
May 14 2015 00:29
hm, maybe i’m just misunderstanding the code
Kevin Modzelewski
@kmod
May 14 2015 00:29
each slot is for a single type, and if it fails it branches to the next slot
Chris Toshok
@toshok
May 14 2015 00:29
it looks like we’ll blow away a given ICSlot if there isn’t a stack frame in it
Kevin Modzelewski
@kmod
May 14 2015 00:29
yeah
because we have a fixed amount of instruction space currently
Chris Toshok
@toshok
May 14 2015 00:30
right but in the gist I pasted, it seems like the slot will flip between i1 and i2’s guards
Kevin Modzelewski
@kmod
May 14 2015 00:30
if we added the ability to allocate new memory for more ic slots we could have more
Chris Toshok
@toshok
May 14 2015 00:30
instead of ever having the ic contain both
Kevin Modzelewski
@kmod
May 14 2015 02:41
quick metaserver update:
I got it to the point that it's trying to connect to databases that I don't have set up in my testing setup
there's a lot of little things that it ran into, which I added to the trello board
and there are some big libraries that it tries to set up
(numpy, ncrypt)
Travis Hance
@tjhance
May 14 2015 02:42
exciting
Kevin Modzelewski
@kmod
May 14 2015 02:43
I think "getting metaserver to start up without crashing" is probably more like 98% of the work of supporting metaserver :(
if we add the condition of not disabling anything
but at least the good news is that I don't think there will be that much afterwards :)
Chris Toshok
@toshok
May 14 2015 03:52
hrm, ugh. the default mro includes the class itself, so we can’t hope to share them across multiple subclasses
we could guard subclass == subclass.mro()[0] and subclass.mro()[1:] is the same as what we’ve seen (somehow)
Kevin Modzelewski
@kmod
May 14 2015 04:42
I wonder if its safe to assume that classes rarely change
Chris Toshok
@toshok
May 14 2015 04:42
i think we should definitely assume that
Kevin Modzelewski
@kmod
May 14 2015 04:42
ie the class objects themselves get modified
Chris Toshok
@toshok
May 14 2015 04:42
and invalidate on change
but the biggest django problem is that the classes are all different :/
they’re all subclassed from the same class, and all the methods/attributes are defined on the base class
well, most of them
Marius Wachtler
@undingen
May 14 2015 19:23
when time travel is available I will go back in time and stop people from using the capi...
Chris Toshok
@toshok
May 14 2015 19:26
Hehe
I wonder how many times we hit the individual django megamorphic ics for the same type
Maybe we can hit the slowpath once per new type and set some state on the type object that the ic uses instead of checking class/mro tuple
And also not rewrite to a new slot for that case
Marius Wachtler
@undingen
May 14 2015 19:36
sorry I don't understand your message. Are you suggesting something like a 'version' field guard instead of the mro guard?
Chris Toshok
@toshok
May 14 2015 19:37
And we could internally represent mros with an array of trees, with right to left traversal of the mro tuple, corresponding to root to leaf tree traversal.
Then we could actually guard on mro_node pointers
Marius Wachtler
@undingen
May 14 2015 19:40
ok I think now I understand it better: the problem is that the current mro guard is a direct pointer comparison which will fail the guard even if the tuple has the same entries. You are suggesting a way to make the guard less specific. correct?
Chris Toshok
@toshok
May 14 2015 19:49
Yeah, like the ic could encode something like "flag is false and parent mro_node is 0x..."
That could allow the same ic slot to work for all subclasses (after 1 trip through the slowpath at least)
Chris Toshok
@toshok
May 14 2015 19:55
Trouble is the tuple won't have the same entries for the TranslateParser. The first element is always different since the class is defined on each closure call
Chris Toshok
@toshok
May 14 2015 20:06
i’m guessing tp_mro can be changed at any time in C extensions though :/
Chris Toshok
@toshok
May 14 2015 20:16
anyway, this just addresses the pattern of methods/attributes defined in the base class
Kevin Modzelewski
@kmod
May 14 2015 20:56
I'm not sure if it'd work to represent mros as trees
given what lengths cpython goes to in order to flatten it and deal with arbitrary inheritance dags
we might be able to guard on tp_bases?
well, if assuming that we made it reuse the same tuple object if the values were the same
but I would worry that we'd be over-matching to this specific case where there's exactly one difference in the inheritance
ex if you created a subclass of those subclasses, it wouldn't work
the "guarding on some representation of the total shape of the class object" seems like it might be more general?
Chris Toshok
@toshok
May 14 2015 21:01
yeah. i’m assuming you mean total shape = including mro/bases
maybe it would be a good idea to flatten the inheritance into some per-class total_shape
Chris Toshok
@toshok
May 14 2015 21:13
wow. increasing number of getattr IC’s from 2 to 302 drops 2 seconds off runtime :)
and that’s with the expected cycles just doing nothing but ~300*512 nops before the patchpoints are full
that dropped 114k rewrite attempts skipped because the ic was megamorphic
so ~100k fewer rewrite attempts = ~2s
potentially, anyway. so we can maybe get another 7 seconds of runtime if all the megamorphic rewrite attempts go away
Marius Wachtler
@undingen
May 14 2015 21:24
and there should still be an increase in runtime because of the ~300 compares and branches...
Chris Toshok
@toshok
May 14 2015 21:24
yes
Marius Wachtler
@undingen
May 14 2015 21:27
sounds like we really should improve the IC / class hierarchy stuff :-). I'm wondering how pypy does it
Chris Toshok
@toshok
May 14 2015 21:30
interesting. bumping up every IC type in patchpoints.cpp to 400+ what it was gets us to 84531 rewriter_skipped_because_megamorphic.
changing it to 500+, it remains 84531
runtime ic’s?
btw at 400+ we’re back up at 30s, so there’s definitely a perf impact to larger ics :)
Marius Wachtler
@undingen
May 14 2015 21:35
http://bugs.python.org/file7940/python26-classattrcache.diff this is the cpython patch for the method cache from http://bugs.python.org/issue1700288 not sure if we could use something like this. but maybe nice for the ast interpreter
Chris Toshok
@toshok
May 14 2015 21:45
oh duh. the threshold has nothing to do with slot counts
so i’ve made 500 slot ic’s that have 400 empty slots :)
Chris Toshok
@toshok
May 14 2015 21:56
that method cache is interesting
essentially like what we’re talking about wrt flattening inheritance
and has the same invalidation concerns
toshok @toshok makes callsite IC’s have 5000 slots
Chris Toshok
@toshok
May 14 2015 22:12
what are the things that influence getattr’s behavior? mro + __getattribute__ on any class in bases/mro?
i need to read more about this
Travis Hance
@tjhance
May 14 2015 22:12
what information does the mro encapsulate?
Chris Toshok
@toshok
May 14 2015 22:13
like, if we can detect that there is no funny business in attribute lookup, we should be able to just use the class’s hiddenclass
lookup order for typeLookup()
Marius Wachtler
@undingen
May 14 2015 22:13
the patchpoints with 5000 slots should even fit a few 64k tech demos :-D
Chris Toshok
@toshok
May 14 2015 22:14
heh code_size was massive :)
err, code_bytes
Travis Hance
@tjhance
May 14 2015 22:14
right but, what exactly is mro?
Chris Toshok
@toshok
May 14 2015 22:14
in the usual case it’s a tuple including the class and all its bases
Travis Hance
@tjhance
May 14 2015 22:15
ah
Chris Toshok
@toshok
May 14 2015 22:15
i.e. str_cls’s mro = (str_cls, basestring_cls, object_cls)
is tp_bases the same except without the class?
like, (basestring_cls, object_cls)?
@undingen: code_bytes: 5375370840
Marius Wachtler
@undingen
May 14 2015 22:18
Good that we lz4 compress the objects before writing to disk
Chris Toshok
@toshok
May 14 2015 22:18
hm, total shape of the class would just be the class’s hidden class + the mro node if mro’s were a tree
128bits
andrewchambers
@andrewchambers
May 14 2015 22:20
Would you guys consider setting up a nightly build download?
Travis Hance
@tjhance
May 14 2015 22:20
you would also want to know what methods the class has, right? like __getattribute__ and __getattr__
and you’d also need to check the class’s field for __get__
so the mro and the hidden class definitely aren’t enough information to know that there isn’t any funny business
we’d need to come up with some format for the “shape” of a class that has all that information
unrelatedly, we should consider renaming ‘hidden class’ to ‘hidden struct’ or something since ‘class’ has too many meanings right now
andrewchambers
@andrewchambers
May 14 2015 22:25
You are using v8 style hidden classes? cool
Travis Hance
@tjhance
May 14 2015 22:40
yep :)
actually… I guess that’s a good reason to not rename it from ‘hidden class'
Kevin Modzelewski
@kmod
May 14 2015 22:46
yeah unfortunately it's kind of a confusing term in a language with actual classes
re nightly builds, I think we could do it, but we haven't worked through the process of producing a distributable result
ie figuring out everything that we need to package up
@denji had #378 which builds tarballs but we need to figure out what to put in them
so anyway, yes we could probably do them once we can distribute builds of any sort :)
mro is a serialization of the inheritance dag
so in the single inheritance case it's just the chain of (cls, cls->tp_base, cls->tp_base->tp_base, ..., object_cls)
Kevin Modzelewski
@kmod
May 14 2015 22:52
in the multiple inheritance case I think it's a topological sorting of the inheritance graph
Travis Hance
@tjhance
May 14 2015 22:53
I feel like we should make a ‘hidden type’ or something which is mro + __getattr__ etc. + all the fields with __get__ or __set__
Chris Toshok
@toshok
May 14 2015 23:05
v8 doesn’t even call them hidden classes :)
well, not in the code, anyway
we could also just abort the creation of the hidden type if we run into __getattr__ / __get__/ __set__
which would force us back through slow paths for those types
Travis Hance
@tjhance
May 14 2015 23:08
idk I feel like that will be pretty common though
and we already have rewriting logic to handle those, though, but the guards have to walk up the hierarchy checking everything
but, maybe that doesn’t actually cost too much
idunno
Marius Wachtler
@undingen
May 14 2015 23:10
it at least makes the patchpoints quite large
Travis Hance
@tjhance
May 14 2015 23:10
oh, that’s a good point
Chris Toshok
@toshok
May 14 2015 23:15
oh cool, we already link classes into superclass list of tp_subclasses
love finding out something I thought I’d have to add is already in :)
Travis Hance
@tjhance
May 14 2015 23:16
I didn’t even realize that existed
Chris Toshok
@toshok
May 14 2015 23:23
are SINGLETON hidden classes the same as NORMAL except that they don’t participate in the tree?
i.e. they allow modification of attribute map without (possibly) creating more HiddenClasses?
Kevin Modzelewski
@kmod
May 14 2015 23:27
yeah, they're unshared so that they can be mutable
Travis Hance
@tjhance
May 14 2015 23:27
when do we mutate them?
is it to add fields without invalidating ICs, or something?
Kevin Modzelewski
@kmod
May 14 2015 23:36
well when we mutate them we have to invalidate any existing ics for them
but it's for objects that we don't expect to have the same shape as anything else
and also potentially have a lot of attributes
so we use it for types and modules currently