Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP]we can use Cython 0.25 without modification now #1374

Open
wants to merge 4,901 commits into
base: master
Choose a base branch
from

Conversation

Daetalus
Copy link
Contributor

No description provided.

undingen and others added 30 commits July 14, 2016 22:10
docker: we are now using cython 0.24 + update readme for 0.5.1 release
it support non str separators
fixes pyston#1297
…slots_issue1197

Add sq_slots & mp_slots to instance (issue pyston#1197)
Internally we store it as either-dict-or-module, but when we expose
it to Python it needs to be given back as a mapping.  I looked briefly
into switching the internal representation, but that seemed like a much
more invasive change.

Also, add a pytest test that exposed this issue.
Have function.func_globals always be dict-like
implement tp_str and tp_repr for more of our builtin types
+ fix a leak in PyList_Insert
add _bisect, always_use_version support CAPI, set some frequent accessed slots
When try to support tk. The Py_None is a wrapper of pyston `None`. It
will conflict with other library's None. So rename the pure pyston
`None` to pyston_None. And use Py_None instead pyston_None when
possible. Just like dropbox#1231 for Py_True and Py_False
Pyston already has Py_IgnoreEnvironmentFlag, just add it to sys.flags.
Replace None with Py_None and some other fixing.
kmod and others added 4 commits September 28, 2016 21:55
Add test for typing.py and fix the issues it uncovers
We fast-pathed "type has __init__ but not __new__" by saying
that the default __new__ would always succeed, but this isn't true for abstract classes.
@kmod
Copy link
Collaborator

kmod commented Oct 4, 2016

I think we should still get this PR in -- how about just undoing the changes to lxml_test.py, but upgrading the others to cython 0.25?

@Daetalus
Copy link
Contributor Author

Daetalus commented Oct 4, 2016

Np, sorry for the delay. I will change it soon.

kmod and others added 21 commits October 4, 2016 22:49
Specifically, for the case that locals==NULL (which Cython exercises), which
wasn't previously working.

now using ctypes to test the c api
A function at risk of "naive misuse", it is only accessible via the C API.

This commit adds basic support for it using the same mechanism we use for
signals.  We also have the GIL-check mechanism, but that would be a bit more
work to get working right now due to the fact that our GIL-checks don't support
throwing exceptions.  Doing the async-exc check during signal checking means
that we will throw the async exc faster than CPython does.  It also means
that there are some pathological cases where with a lot of threads and a lot
of async excs we will probably have much worse performance.  But as long as they
are rare I think this commit shouldn't add any steady-state performance costs.
Too many things want this to be able to remove it completely.  Surprisingly,
having imp.get_magic exist but return a changing value seems to make all those
libraries happy.
We weren't properly reducing the stack depth when yielding from a generator,
meaning that the recursion depth was effectively decreased by the number
of active generators.
google's protobuf library gives away one too many refs to one of its types.

This workaround feels excessively specific, but this is a common mistake to
make, and it works just fine in CPython, so let people get away with it
as well in Pyston.
Specifically, when passed to PyArg_ParseTuple when a dict argument
is asked for.
**basic design:**
This PR changes our BST nodes to directly operate on vregs instead of pointers to other nodes and names (except a few exceptions: `BST_Invoke`, `BST_MakeFunction` and `BST_MakeClass` which still needs to get converted).
Most nodes got a destination vreg and one or more source vregs. Currently all of them are 32bit long but I plan to store them more compact very soon. Some nodes support a variable size of operands (e.g. the tuple node) but the size can't change after creating the node. I removed several unneeded opcodes and split a lot of nodes into separate opcodes (it may make sense to split them even further in the future).
Generally all instructions except `CopyVReg` kill the source operand vregs except if the source is a ref to a constant. If one needs the preserve the source vreg on needs to create a new temporary using the `CopyVReg` opcode.

There is a special vreg number: `VREG_UNDEFINED = std::numeric_limits<int>::min()`.
- when it's set as an operand vreg: it means that this is a not-set optional argument. (e.g. for a slice which only has `lower` set, `upper` would be `VREG_UNDEFINED`)
- if it's the destination it's means the result value should get immediately killed (e.g. `invoke 15 16: %undef = %11(%14)` this is a call whose result gets ignored)

all other negative vreg numbers are indices into a constant table (after adding 1 and making them positive).
(e.g. `(4, 2, 'lala')` generates:  `%undef = (%-1|4|, %-2|2|, %-3|'lala'|)` this creates a tuple whose elements are the constant idx -1, -2 and -3. In order to make it easier for a human to understand we print the actual value of the constant between | characters)
- constants can be all str and numeric types and 'None'.
- every constant will only get stored once in the table

this reduces the total memory usage by about 20% currently but I'm very sure with the future changes it will be significantly lower.

**near future:**
- change the jump and branch instruction to reference `CFGBlocks` by index.
- store all `InternedString` inside a table and use indices into the the table to access them.
- remove the 'BoxedCode*' member
- devirtualize the classes
= with this changes the bytecode can get freely copied around (only need to update the CFGBlock table) which allows us to attach the directly next to each other.

- I plan to use one bit of the the opcode to mark the instruction as only requiring 8bit vreg operands (which should handle the majority of cases with 128 temps and 127 constants + 1undef vreg value)
- another bit will get used to specify if this instruction is inside an `invoke`. if this bit is set there are 2 one 1 or 4 bytes long block indices directly behind the instruction.

- serialize the bytecode to disk. (maybe serialize the constants using pickle)

**thing which need to get improved**
- currently the constant table get's attached to the `BoxedModule` maybe there is a better location, I also needed to pass the `BoxedModule` into some functions e.g. BST printing because otherwise we could not pretty-print the constants
- `BST_Name` is not an opcode it's just used to initialize the arguments when a function get's called and stores where and how the arguments need to get stored.
- more consistent opcode names and rename `TmpValue` to something better
- we currently don't print the `InternedString` name - we only print the vreg number

**additional changed made which are hidden in the large diff** 👎
- removed unused code initializing the items of `BST_Dict` (we use/used separate  assignments to add the items)
- lower `ExtSlice` inside the CFG phase to a tuple of slices
- separated opcode for load subscript when it needs to be a slice and when it's only lower and upper (=`__getslice__`) before this got handled in the interpreter/jit
- generate a constant `None` load inside the CFG when `None` gets loaded by name
BST: convert all nodes to directly operate at vregs instead of names
- Fix a test to do the correct number of iterations
- Catch more exceptions in PyDict_GetItem/createModule (now that more things can throw)
This warning is quite annoying right now
A number of small fixes from the dropbox testsuite
move constants into CodeConstants, call BoxedCode destructor, cleanup BST nodes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants