These are chat archives for dropbox/pyston

21st
Jul 2015
Travis Hance
@tjhance
Jul 21 2015 00:20
maybe because pgo wasn’t ‘guided’ by that benchmark?
Chris Toshok
@toshok
Jul 21 2015 00:20
I modified the Makefile to run that benchmark while building up the profile data :/
Travis Hance
@tjhance
Jul 21 2015 00:20
oh
Chris Toshok
@toshok
Jul 21 2015 00:21
not a tiny different either. 0m4.148s for pyston_release, 0m4.502s for pyston_release_gcc_pgo
Chris Toshok
@toshok
Jul 21 2015 00:28
it would be interesting to try and get django itself running against pyston again
see how we do with apache bench, etc
Chris Toshok
@toshok
Jul 21 2015 00:36
ugh, looks like the pgo build isn’t linking to jemalloc?
the rpath is there, but no jemalloc
Chris Toshok
@toshok
Jul 21 2015 01:03
a cautionary pgo tale (windows specific, but still): https://bugzilla.mozilla.org/show_bug.cgi?id=1167883#c24
Chris Toshok
@toshok
Jul 21 2015 01:08
are malloc and free called from IC entries?
Travis Hance
@tjhance
Jul 21 2015 01:10
I don't think so
Unless the baseline jit does
But none of the objmodel.cpp stuff does
Chris Toshok
@toshok
Jul 21 2015 01:11
ah, OSR frames do it looks like
err // Leave in the ability to use malloc but I guess don't use it.
it’s not a bad for malloc, but free is a pain: perf shows malloc and free taking 6% of execution time total (~3% each.) of the 3% in free, 60+% of that time is unaccounted
lots of
   - free41.63% 0x6                                      ▒
      + 11.98% 0x7f145c400158                           ▒
        7.60% 0x1ffffffff                               ▒
      + 2.28% pyston::Rewriter::~Rewriter()             ▒
        2.09% 0x2ffffffff                               ▒
Sun
@Daetalus
Jul 21 2015 06:34
Travis clang debug mode checks failed. Am I alone?
Travis Hance
@tjhance
Jul 21 2015 06:37
I think the build is broken for master right now
all the PRs are having similar failures
Sun
@Daetalus
Jul 21 2015 07:23

Hi, I want to address #598. Let min and max support keyword argument. I let min to receive a BoxedDict* kwargs. And want to extract the function from it then use runtimeCall. But I don't know how.

CPython use PyArg_ParseTupleAndKeywords. I test it but didn't work. I also use

static BoxedString* key_str = internStringImmortal("key");
auto it = kwargs->d.find(key_str);
Box* key_func = it.second;

But didn't work, could not use the key_func in runtimeCall.

Could anyone give some helps or hints? Thanks!

Travis Hance
@tjhance
Jul 21 2015 07:26
Why can’t you use key_func in the runtimeCall? What happens when you try?
Also, pyston handles arguments differently from cpython, to avoid creating extra dict objects
Marius Wachtler
@undingen
Jul 21 2015 07:27
I think you should not need PyArg_ParseTupleAndKeywords just set takes_kwargs = true when registering the func with boxRTFunction
Travis Hance
@tjhance
Jul 21 2015 07:28
I don’t think min wants takes_kwargs though, from the looks of it
it wants to take a default (“key”)
mmm
I might be wrong here
Sun
@Daetalus
Jul 21 2015 07:30
Kevin suggest me to use takes_kwargs on a private email. I just follow his instructions :smile: I try to use ParamNames before, but failed.
Travis Hance
@tjhance
Jul 21 2015 07:30
because the key param goes after the varargs, so it can’t be a default
so it must be a kwargs dict
yes, marius and kevin are correct
Sun
@Daetalus
Jul 21 2015 07:32
Please hold on a second. I will try the runtimeCall again. Because I write it several days ago and working on #719 recently.
Travis Hance
@tjhance
Jul 21 2015 07:32
anyway… what you have for getting key_func out of the dictionary looks fine
Marius Wachtler
@undingen
Jul 21 2015 07:32
is the kwargs dict set or is it empty?
Travis Hance
@tjhance
Jul 21 2015 07:40
why are the same pyston.so’s found in both lib_pyston/ and in build/Debug/lib_pyston/? which are used?
Sun
@Daetalus
Jul 21 2015 07:47
Ok... It could work know. I don't remember why it not worked last time. Thanks for the helps!
Kevin Modzelewski
@kmod
Jul 21 2015 09:55
yeah 'key' is a keyword-only param so it has to go through kwargs, it's weird
I think the .so's in build/ get used
actually I don't remember
I think the ones in lib_pyston are old
I don't have any in mine
we should figure out a good way of cleaning out that directory, since if you have cruft in there it will silently get used
Sun
@Daetalus
Jul 21 2015 11:13
@kmod Please review #719 only if you have time, thanks!
Travis Hance
@tjhance
Jul 21 2015 15:21
Yeah they were silently being used, I think, which caused trouble
Marius Wachtler
@undingen
Jul 21 2015 16:47
I would never have thought that release builds can work so much better than the debug ones... at least on travis-ci..
Travis Hance
@tjhance
Jul 21 2015 16:52
Well its cause the release versions don't have the asserts, yeah?
Marius Wachtler
@undingen
Jul 21 2015 16:56
could be but wouldn't I see the assert text if it's an assert? e.g.: https://travis-ci.org/dropbox/pyston/jobs/71942458
Chris Toshok
@toshok
Jul 21 2015 17:20
what’s exit code -114?
weird, #709 doesn’t seem to have travis-ci output
Marius Wachtler
@undingen
Jul 21 2015 17:23
strange
Marius Wachtler
@undingen
Jul 21 2015 18:01
mmh ok I hit again a -114... don't know what this error wants to tell me
Rudi Chen
@rudi-c
Jul 21 2015 18:02
It seems to show up inconsistently - doesn't happen on #536 for example
Marius Wachtler
@undingen
Jul 21 2015 18:09
I'm somewhat happy that you encounter the same issue because I started question my PRs...
Chris Toshok
@toshok
Jul 21 2015 18:32
i’ve seen -114 in a couple of mine too
Marius Wachtler
@undingen
Jul 21 2015 18:38
I'm excited about the doc change build, does this one fail too?
Chris Toshok
@toshok
Jul 21 2015 18:39
i hope not (at least I hope changing doc files doesn’t cause further problems :)
Travis Hance
@tjhance
Jul 21 2015 18:40
well, the pyston executable actually reads the doc files to figure out how pyston is supposed to work
so modifying the doc files can definitely affect its results
Chris Toshok
@toshok
Jul 21 2015 18:40
heh, moonlight’s CI build would parse the README :)
Travis Hance
@tjhance
Jul 21 2015 18:41
uh
Chris Toshok
@toshok
Jul 21 2015 18:41
for git hashes + svn revisions, etc
we were using svn still for the repo (though all of us used git-svn), so no git submodules
Rudi Chen
@rudi-c
Jul 21 2015 18:42
How should I go about trying to install NumPy and investigating crashes?
Travis Hance
@tjhance
Jul 21 2015 18:43
this is what pyston used to do, except it didn’t save the hashes in the README
Chris Toshok
@toshok
Jul 21 2015 18:44
scary: NEEDED_MONO_VERSION:=$(shell grep 'cd mono .*hard' $(PWD)/README | awk -F' ' '{print $$7}’)
@rudi-c: you could do a virtualenv setup, or probably run something like …../pyston_dbg setup.py inside numpy
Marius Wachtler
@undingen
Jul 21 2015 18:47
oh wow sorry, I jsut wanted to point you to the lxml test which sets up the patched cython
but I jsut saw that I never finished this one / submited it :-(
Rudi Chen
@rudi-c
Jul 21 2015 18:49
Is the patch to fix
ImportError: No module named Cython.Compiler.Main
Traceback (most recent call last):
  File "/mnt/rudi/numpy/tools/cythonize.py", line 199, in <module>:
    main()
  File "/mnt/rudi/numpy/tools/cythonize.py", line 195, in main:
    find_process_files(root_dir)
  File "/mnt/rudi/numpy/tools/cythonize.py", line 187, in find_process_files:
    process(cur_dir, fromfile, tofile, function, hash_db)
  File "/mnt/rudi/numpy/tools/cythonize.py", line 161, in process:
    processor_function(fromfile, tofile)
  File "/mnt/rudi/numpy/tools/cythonize.py", line 81, in process_pyx:
    raise Exception('Cython failed')
Exception: Cython failed
Also I'm guessing I need to install Cython?
Marius Wachtler
@undingen
Jul 21 2015 18:52
I'm not sure what bugs it fixes but I created the a gist for the patch
And yeah you have todo something like this:
    url = "http://cython.org/release/Cython-0.22.tar.gz"
    subprocess.check_call(["wget", url], cwd=SRC_DIR)
    subprocess.check_call(["tar", "-zxf", "Cython-0.22.tar.gz"], cwd=SRC_DIR)
    CYTHON_DIR = os.path.abspath(os.path.join(SRC_DIR, "Cython-0.22"))

    PATCH_FILE = os.path.abspath(os.path.join(os.path.dirname(__file__), "Cython_0001-Pyston-change-we-don-t-support-custom-traceback-entr.patch"))
    subprocess.check_call(["patch", "-p1", "--input=" + PATCH_FILE], cwd=CYTHON_DIR)
    print "Applied Cython patch"

    subprocess.check_call([PYTHON_EXE, "setup.py", "build"], cwd=CYTHON_DIR)
    subprocess.check_call([PYTHON_EXE, "setup.py", "install"], cwd=CYTHON_DIR)
and than you can try builing numpy with the virtualenv
I tried it once, it didn't work but I came AFAIK much further with the applied patch
Chris Toshok
@toshok
Jul 21 2015 18:59
does travis-ci offer docker images or something that we can load up and replicate their build hosts somehow?
Kevin Modzelewski
@kmod
Jul 21 2015 19:16
I think -114 is a sigalrm
I don't know why that happens though
Chris Toshok
@toshok
Jul 21 2015 19:20
i thought it was just -$signal_number. do we add -100 someplace else?
Kevin Modzelewski
@kmod
Jul 21 2015 19:21
well
there are a bunch of places we mangle the exit code :)
the gdb watcher does it
Chris Toshok
@toshok
Jul 21 2015 19:21
yeah just saw that
Kevin Modzelewski
@kmod
Jul 21 2015 19:21
and then the tester script does it too
my theory is that it's a sigalrm (14) which the gdb watcher returns as 142 (128+14) which the tester interprets as -114
haven't looked into it though
I did some testing and I was able to get gdb to hang
it might be some unrelated crash, and then the gdb watcher hangs since it can't understand the bjit frames
Chris Toshok
@toshok
Jul 21 2015 19:23
yeah, that definitely explains -114. +128 then -256 since 142 >= 128
Kevin Modzelewski
@kmod
Jul 21 2015 19:23
and then the safety sigalrm comes in and kills it
Chris Toshok
@toshok
Jul 21 2015 19:38
there’s also a lot of really crazy variability in test duration on travis-ci it seems
platform_test.py Correct output (13434.4ms)
Marius Wachtler
@undingen
Jul 21 2015 20:05
mmh looks like that the debug build error is gone in the last few travis-ci builds... but I don't think anything changed?
Chris Toshok
@toshok
Jul 21 2015 20:24
spent some time reading about v8’s IC’s today. they do an interesting thing where their baseline compiler emits IC’s out of line from the rest of the code. The slowpath adds a new IC stub (potentially reallocating the IC, and rewriting the call target in the jitted code)
their optimizing compiler will fold the entire IC back into the method if it’s small enough
(and if there’s only one target, will also sometimes inline the operation/target function entirely)
they have things a little easier since they’re single threaded, but a system like that would work for pyston too if instead of a direct call to the IC there was a load from a thread local vector of IC targets + call
Travis Hance
@tjhance
Jul 21 2015 20:29
what’s the advantage of doing it this way?
Marius Wachtler
@undingen
Jul 21 2015 20:31
#2005 encountered the multiprocessing_test.py error :-(
Chris Toshok
@toshok
Jul 21 2015 21:04
@tjhance: smaller jitted code, smaller ic’s for monomorphic/infrequently hit ic's
the TLS stuff, no clue
Marius Wachtler
@undingen
Jul 21 2015 21:09
and switching between tiers should save one slowpath call + rewrite...
Marius Wachtler
@undingen
Jul 21 2015 21:20
FTL uses inline ICs but switches to external if code exceeds the inline size: http://trac.webkit.org/changeset/185772
but the had to add the external IC storage because of correctness not because of perf
very few IC types
Kevin Modzelewski
@kmod
Jul 21 2015 21:48
hey @undingen I canceled some or your travis-ci builds
so that I can get the "fix the build" builds to go through
(we're getting rate limited right now)
they will probably fail anyway without the timeout-increase :(
just an fyi to everyone that I might have to do this to your builds as well :/
Marius Wachtler
@undingen
Jul 21 2015 21:51
ok
@thoshok and a lot smaller than ours..
Marius Wachtler
@undingen
Jul 21 2015 22:09
do we have type speculation disabled? becasue it looks like we never deopt...
Kevin Modzelewski
@kmod
Jul 21 2015 22:11
yeah I think it's currently disabled
Chris Toshok
@toshok
Jul 21 2015 22:43
are there magic objdump flags to output the eh_frame for a given method?
Kevin Modzelewski
@kmod
Jul 21 2015 23:10
you can try readelf -w pyston_dbg
Chris Toshok
@toshok
Jul 21 2015 23:11
well, that definitely gives pc ranges in a more useful format than objdump
Marius Wachtler
@undingen
Jul 21 2015 23:20
why do you look at eh frames? :-D
Chris Toshok
@toshok
Jul 21 2015 23:20
pgo is doing something wonky with unwinding :/
Marius Wachtler
@undingen
Jul 21 2015 23:20
:-(
Chris Toshok
@toshok
Jul 21 2015 23:40
so yeah, this looks like a compiler bug.. instructions are inserted but the eh frame isn’t modified at all
the eh_frame has the same advance_loc’s in both pgo and non-pgo builds. 6, 3, 71, and 2
advance_loc 3 and 4 correspond to the retq and the instruction following the retq in the non-pgo build
but in the pgo build 3 doesn’t correspond to the retq anymore. it’s the addq for the gconv counter
Chris Toshok
@toshok
Jul 21 2015 23:50
ah, the advance_loc’s are different in the #4 slot: 2 in non-pgo, 9 in pgo. so that keeps the final info correct and corresponding to the same instruction. but #3..
Rudi Chen
@rudi-c
Jul 21 2015 23:50
gcc: build/src.linux-x86_64-2.7/numpy/core/src/umath/scalarmath.c
gcc: numpy/core/src/umath/ufunc_object.c
numpy/core/src/umath/ufunc_object.c: In function ‘ufunc_generic_call’:
numpy/core/src/umath/ufunc_object.c:4348:1: error: unrecognizable insn:
 }
 ^
(insn 1120 818 819 114 (set (reg:DI 3 bx [367])
        (sign_extract:DI (mult:DI (subreg:DI (reg/v:SI 0 ax [orig:108 i ] [108]) 0)
                (const_int 8 [0x8]))
            (const_int 35 [0x23])
            (const_int 0 [0]))) numpy/core/src/umath/ufunc_object.c:4345 -1
     (nil))
numpy/core/src/umath/ufunc_object.c:4348:1: internal compiler error: in extract_insn, at recog.c:2154
Please submit a full bug report,
with preprocessed source if appropriate.
See <file:///usr/share/doc/gcc-4.8/README.Bugs> for instructions.
Preprocessed source stored into /tmp/cc1TRHk1.out file, please attach this to your bugreport.
derp I crashed gcc
Chris Toshok
@toshok
Jul 21 2015 23:51
let’s form a gcc 4.8 fan club
although "unrecognizable insn” from a compiler. awesome
Rudi Chen
@rudi-c
Jul 21 2015 23:52
Nice lisp-like output too
Chris Toshok
@toshok
Jul 21 2015 23:52
stallman’s influence?
Marius Wachtler
@undingen
Jul 21 2015 23:54
@toshok you could also try to compile the file containing the checkUnpackingLength with -S. The emitted assembler output contains the EH directives and should be easier to read
Chris Toshok
@toshok
Jul 21 2015 23:55
ah, right good call
i also noticed that gcc was still putting a retq at all
raiseExcHelper is __attribute__((__noreturn__)), but iirc gcc uses ((noreturn))
oh, nevermind. bizarre. that’s the retq for the %rdi == %rsi case.
wonder why it put that there