These are chat archives for dropbox/pyston

24th
Jun 2015
Chris Toshok
@toshok
Jun 24 2015 00:30
(gdb) print list
$1 = (PyWeakReference **) 0xbbbbbbce2cad47c3
Chris Toshok
@toshok
Jun 24 2015 00:54
so it appears we have other fallout from the fact that we don’t keep classes alive long enough
and we’re just getting lucky
that bad list ptr comes about because I added the 0xbb memset to SmallArena::free, which is what we use to free objects after we’ve processed their weakrefs
so the class ends up in the weakly_referenced list first, we process it, memset it to 0xbb, then hit an instance of that class (o), and PyWeakReference** list = (PyWeakReference**)PyObject_GET_WEAKREFS_LISTPTR(o); ends up pointing to (char*)o + 0xbbbbbbbbbbbbbbbb
tl;dr: gc zero-on-free has to wait until after @rudi-c’s finalizer work :)
Rudi Chen
@rudi-c
Jun 24 2015 00:57
So here we'd want to either keep the class alive longer or store the Weakrefs_Listptr on the instances?
Chris Toshok
@toshok
Jun 24 2015 00:58
yeah, either of those work, although we don’t need the actual listptr, just the offset
Rudi Chen
@rudi-c
Jun 24 2015 00:59
Oh here's another idea : never free classes, keep count of the number of classes around and have a separate GC pass just for classes.
Chris Toshok
@toshok
Jun 24 2015 00:59
PyObject_GET_WEAKREFS_LISTPTR(o) I think just does (char*)o + o->cls->tp_weakreflist
Rudi Chen
@rudi-c
Jun 24 2015 01:00
Also, all classes the same size => they can go in their own heap/SmallArenaBlock or whatnot.
Chris Toshok
@toshok
Jun 24 2015 01:00
i’m not sure that’s a win. we could do something really simple for the weak ref case: if its cls == type_cls, append it to the vector. otherwise prepend
another thing we could do is just make two passes over the list. pass 1: process weak refs. pass 2: actually free
Rudi Chen
@rudi-c
Jun 24 2015 01:03
Wait what's the list? Just weakly_referenced?
Chris Toshok
@toshok
Jun 24 2015 01:03
yeah
in this particular case
Rudi Chen
@rudi-c
Jun 24 2015 01:04
I'm not sure I see how it affects the problem of classes getting freed too early (except for a weakly-referenced class object).
Chris Toshok
@toshok
Jun 24 2015 01:05
right - it’s just a specific instance of the same general problem (the class should be freed after instances to it)
we were just getting very lucky with the weakly_referenced list because we weren’t memsetting them to garbage value like we were for non-weakly_referenced objects
so even though the classes were destroyed first, their fields still had valid contents
hm, even doing both of those things (append for types, prepend for all else; 2 passes) it still crashes with 0xbbbbbbbbbbbbbbbb
Chris Toshok
@toshok
Jun 24 2015 01:10
just in other places
Rudi Chen
@rudi-c
Jun 24 2015 01:10
We could also store a tuple (Box, offset) in the weakly_referenced list instead of just the Box.
Chris Toshok
@toshok
Jun 24 2015 01:11
yeah, but unfortunately the _doFree that we end up in checks tp_dealloc :/
suffice to say, i’ll wait until you’ve figured out the other ordering constraints :)
we’re already going to have to order instance + class destruction for finalization, right?
Rudi Chen
@rudi-c
Jun 24 2015 01:14
There's still some approaches I haven't tried that could remove the need to look at classes during the sweep phase, involving partially rooting objects with simple destructors and calling them outside the sweep phase. It may help with performance too (or make it worse).
Marius Wachtler
@undingen
Jun 24 2015 21:28
@kmod what's reason for the revert back to gettimeofday from the ticks?
Chris Toshok
@toshok
Jun 24 2015 21:29
the math was all wrong for Timers after that change
and the resulting output wasn’t being converted to us_ like it should have been
Marius Wachtler
@undingen
Jun 24 2015 21:35
oh k
we could also try clock_gettime which may have less overhead than gettimeofday???
Chris Toshok
@toshok
Jun 24 2015 21:40
it’s still a syscall unfortunately
yeah should not make much difference