yeah, Python has a bunch of rules that depend on whether or not a class was heap-allocated
and we mostly ignore that logic
usually if CPython wants to use tp_as_sequence, they statically-allocate a separate PySequencMethods struct and then put that pointer into the PyTypeObject struct
right
but I feel like it should be ok to make them static BoxedClass's (non-Heap)
but yeah we can't depend on tp_as_sequence getting initialized by the BoxedHeapClass constructor
so what’s the right way to initialize it, then?
<jasone> Holy crap that compiled fast, toshok
<jasone> I was prepared to wait over an hour.
err well I'm not sure what the rest of it looks like
but there shouldn't be any reason it couldn't just work the same way say, the unicode type does
won’t we then need to visit them conservatively?
well we probably shouldn't call PyType_Ready on them
but just in terms of getting the data structures allocated and initialized, that could happen the same way
oh but I guess we would want to call our constructor on it
well, I can fix the perf regression caused by the 1 type registered by the grp module
by removing the non heap root machinery entirely :/
hm if the non heap root stuff is slow, that doesn’t bode well for the statically allocated type objects :(
might be able to do something about it by having another layer - min/max/unordered_set per .so (and one for the executable)
there’s very little that actually relies on the nonheap-root status (other than for assertions)
oh wait so what’s the issue?
we have ~150 non-heap roots for django template
they’re in the range 0x1600000-0x17f0000 or something
every time we GCVisitor::visit a pointer we check if it’s in that range (we keep min/max), and if it is, we look in the unordered_set to make sure it was registered
the grp .so registers a nonheap root that’s at like 0x2a000000000
or something really high
actually inside gdb it gets mapped up at 0x7fffff….. which means we look up every pointer in the unordered_set before then looking in our actual heap for it
I didn’t realize gdb affected where stuff got mapped
could we just make these checks into asserts?
yeah, going to reorder things such that we check our heap bounds first
then we can assert
trouble is, that 0x2a…. address, though. that could have been in the large heap
which is a problem :/
think I’m going to make the arenas just reserve their entire mappings at startup
how do I re-throw an exception? I tried a bare throw and I got
__cxa_rethrow() unimplemented; please don't use barethrow' in Pyston!`
should I just do
throw e where e is the exception object I caught?
yeah
k cool
do tpp_* functions throw exceptions?
yeah
how is current progress in pyston ? can i use it now in production ?
Pyston progresses very rapid. we are now able to run large applications with minor problems. And getting new large libs to run if they don't work out of the box it's mostly a one day job. But main focus is currently on performance again. But as of use in production: I wouldn't recommend using it yet in production. It coming closer but I'm pretty sure if you run it longer you will encounter issues.
How many people work for Pyston as full time job?
~5 people
Thanks!
@kmod what tests are failing / how can I reproduce the issue?
doh it looks like they've been failing for a while
I don't know why I didn't get any emails
at least on master, I've been getting failures with
make run_release_gcc_babel_test
(you need to pull the thing I just pushed to master that adds release_gcc)
@kmod , please take a bit time to look at #683, because I almost rewrite pow functions in
Another question, in
I already implement these 'xxx' stuff in my local repos. But I need to improve it along with oldstyle `_pow` support.
int and long object. The test rely on convert long to float.Another question, in
test/cpython/test_pow, def test_bug643260, due to old-style class not support __rpow__ yet, I change the class to new style class. Is that OK?I already implement these 'xxx' stuff in my local repos. But I need to improve it along with oldstyle `_pow` support.
ok I will take a look :)
I already implement '__xxx__` in old style class, sorry for the typo.
ok it seems to be the interpreter gc change
that's what Travis-CI says was the first red build
and if I revert it, the babel test passes again
I think we might have to pass the wrapper around
instead of passing the interpreter pointer around
if we only have the interpreter pointer, then it won't trace through to see the wrapper and keep the interpreter alive
just a theory, the babel failure is pretty obscure
man, this grp.so thing is making no sense at all
unchanged, the addition of grp.so causes GCVisitor::visit time to spike in perf output (to 1450, 8.9% of samples) due to every single gc heap pointer also being in the range of min-max_nonheap_root
so I made this change: gist.github.com/toshok/a8edfebfe09da0d7bdaa
oh also note before that change, SmallArena::allocationFrom is at 126/0.78%
after that change, GCVisitor::visit drops to 298,1.88%, but SmallArena::allocationFrom jumps to 898/5.70%
and overall there’s barely a reduction in runtime
with that change I would expect GCVisitor::visit to take slightly more time than it did before (it has to do 7 comparisons - 2 for each arena, then one for the null check - instead of just 2 for the min-max non heap roots)
but i wouldn’t expect any additional time spent in SmallArena::allocationFrom
The current work of Pyston is focus on performance?
even removing grp.so (and the others ones I added for this PR) doesn’t help the situation
@Daetalus yes
because small_arena.contains() should return false for this cases. so there shouldn't be any additionals allocationFrom calls??
correct
Thanks, I want to make some contributions, start from let more tests could pass. And already in progress.
Getting more tests to pass is always very welcome :-)
But the PR feedback/review seems slow...
btw: guys sorry for breaking the build. didn't realize the commit was causing the problem
more fun: changing Arena::contains to return true for anything inside of the constant range of
[start, start+size) instead of doing a load of cur (the current high watermark) + comparison, then doing the cur compare inside of each ::allocationFrom drops it from 5.70% to 5%, but ups the cycles to 946.
the
div instruction in SmallArena::allocationFrom (from int obj_idx = offset / size;) shows as taking 50% of the time we spend in that method
Sorry @Daetalus! that's my bad.
also, not sure if this was happening here, but one thing that surprised me about github (but I guess it makes sense) is that it doesn't send out notifications when you update the PR
Oh interesting. So whenever we update a PR, we should always comment?
if it's ready for review again
also, marius's blog post is now live!
yeah, lack of notifications is a pain :/ it does show as synchronized in the gitter activity feed (which scrolls by too quick), and the PR list can be sorted by most/least recently updated, but neither of those are a substitute
:-)
I think one thing that makes it tricky is that not all updates signify that the PR is ready for review again
ex if the build fails
I was thinking of introducing a labeling system so that it could be more clear what state things are in (or maybe more importantly, what state other people thing things are in)
ex things start off as "pending-review", then after review go back to "pending-updates", etc
I thought it would be too heavyweight but maybe it's worth it?
I think it's a good idea to try. It would help with the communication issue (the "am I supposed to be doing something right now?" for both parties).
@kmod #683 updated and pass the Travis Cl test.
merged :)
ok I submitted the blog post to HN
Thanks @kmod ! Another question, I am working on
pow issue in test/cpython/test_pow, function def test_bug643260(self):, due to the old-style class lack of \_\_rpow\_\_ support. It could not pass the test. Could I change it to new style class in order to let the pow test pass?
oh hmm
I am not a huge fan of that :/
could we do the pow changes for now without re-enabling the test_pow test
and then add old-style class rpow?
Good idea.
I am already add old-style class rpow(along with other operators) in my fork. But it seems need some improvements.
I will create a PR tomorrow with the pow changes first.
awesome :)
yeah it would be easier to just change the test, but I think we have to be quite against that so that we can later advertise that we pass the cpython test suite
According to the release interval, 0.4 comming soon?
hopefully! :)
I'd like us to hit some performance targets, but that's taking longer than we expected
Hope I have enough time to let more tests pass before 0.4 release.
Also, I went through the PRs and tagged the ones that I think I'm reviewing along with what I think the status is
though it looks like people without write-access to the repo don't have the ability to change the labels
or assign a reviewer
so we'll see how well this works out
so the workflow for submissions is something like: after a PR passes travis-ci and it’s ready for review, the submitter either adds the label (if they have write access), or requests review?
yeah
I imagine it'll mostly be as-needed
if the pr passes ci, I'll usually review it right away
err, if I get to it :P
looking at the blog-post stats showed me this interesting site:
scared to click
oh wow, much better
mmh I'm getting now a lot of asserts:
if ((uintptr_t)p < SMALL_ARENA_START || (uintptr_t)p >= HUGE_ARENA_START + ARENA_SIZE) {
ASSERT(isNonheapRoot(p), "%p", p);
return;
}
problem is that we called visit with 0 pointers. and they fail the isNonheapRoot check
yeah
dropbox/pyston#701
:-)
sorry about that :/
this babel ASTInterpreterGCWrapper bug is really annoying.
hm, so the pure python impl of mercurial works, but compiling the native modules fails due to our use of
bool in cpython headers
well, “works”, i haven’t really done much testing
so we are getting a mercurial benchmark? :-P
i was going to look into it :)
this might take too long…
mercurial-3.4.1-install/bin/hg clone https://hg.mozilla.org/mozilla-central/ firefox
- we should build it just to make sure the checkout was successful :-D
haha. that would likely swallow up any perf difference between cpython and pyston too
we got >1k views on the blog post :)
oh, one of those was me
how long has it been up?
just a few hours?
yeah
it’s on the front page of slacker news right now
cringe so mercurial has
RES 8.743g so far
what does that mean?
i don’t know how much memory mercurial normally uses when doing a clone (with cpython), but that’s the resident set for pyston running mercurial
oh that’s the memory usage
yeah
first comment on hackernews: python3
we clearly need "when is python3 happening" pinned to our front page :P
damn:
transaction abort!
rollback completed
abort: connection ended unexpectedly
clearly I need a small repo to clone :)
python3: the intern project
concerning the ASTInterpreterGCWrapper I still don't understand why I have to pass the wrapper around every where (but the issue is fixed when I pass it to the baseline jit helper methods etc..)
Because I always allocate the ASTInterpreter instance on the stack. this means the ASTInerpreter::wrapper field should also be on the stack and should get scanned therefore. I don't pass the wrapper around further down but I thought that's not necessary because a few caller stack frames higher up there will always be the ASTInterpreter instance including the wrapper laying around on the stack...
Because I always allocate the ASTInterpreter instance on the stack. this means the ASTInerpreter::wrapper field should also be on the stack and should get scanned therefore. I don't pass the wrapper around further down but I thought that's not necessary because a few caller stack frames higher up there will always be the ASTInterpreter instance including the wrapper laying around on the stack...
oh hrm yeah
maybe I was off with that theory