[WIP]Scikit-learn commit #1308

Daetalus · 2016-07-27T14:55:42Z

In CPython list sort function, before the list get sort, it will reverse the list itself first. See here: https://github.com/python/cpython/blob/2.7/Objects/listobject.c#L2115

This will cause the incompatibility in below:

c = [(0, 0, 0), (1.0, 2, 0.0), (1.0, 3, 0.0)]
print("Original:")
print(c)
print("result:")
d = sorted(c, key=lambda x: x[0], reverse=True)
e = sorted(c, key=lambda x: x[0])
f = sorted(c, reverse=True)
print("Reverse with key:")
print(d)
print("Only has key:")
print(e)
print("Only has reverse")
print(f)

Pyston:

Original:
[(0, 0, 0), (1.0, 2, 0.0), (1.0, 3, 0.0)]
result:
Reverse with key:
[(1.0, 3, 0.0), (1.0, 2, 0.0), (0, 0, 0)]
Only has key:
[(0, 0, 0), (1.0, 2, 0.0), (1.0, 3, 0.0)]
Only has reverse
[(1.0, 3, 0.0), (1.0, 2, 0.0), (0, 0, 0)]

CPython && PyPy:

Original:
[(0, 0, 0), (1.0, 2, 0.0), (1.0, 3, 0.0)]
result:
Reverse with key:
[(1.0, 2, 0.0), (1.0, 3, 0.0), (0, 0, 0)]
Only has key:
[(0, 0, 0), (1.0, 2, 0.0), (1.0, 3, 0.0)]
Only has reverse
[(1.0, 3, 0.0), (1.0, 2, 0.0), (0, 0, 0)]

The differences is in Reverse with key.

CPython use TimSort, this algorithm need the list is descending. So for maintain the stability in reverse sort, it need to reverse the list first, then reverse again when the list got sorted.

So in order to let Pyston compatible with CPython. I add the second commit.

That's just my investigation, @undingen @kmod please correct me if I was wrong. Thanks!

References:

TimSort Java Implementation
TimSort CPython Implementation

string is special in that it is a c++ type which has tp_as_number and tp_as_sequence. This causes problems because when we fixup the slot dispatcher we will set the tp_as_number fields but not the tp_as_sequence because setting both can cause problems. Some extensions (e.g. numpy) require that we use the sq_* functions instead of nb_*. Therefore clear the tp_as_number fields (except nb_remainder which cpython has set too because it is not part of tp_as_sequence).

str: use tp_as_sequence instead of tp_as_number

that is just based off of pulling our latest release

PyObject_New: register the type if the type is not yet registered

The problem is that we emit an llvm "unreachable" instruction, and then continue to emit other code, which fails the verifier. endBlock(DEAD) is supposed to be the right way to handle that, but there is some more work that would need to be done there to get that working properly. So just do the easy thing for now -- create a new BB so that it's ok to emit more code.

We were doing a "call bumpUse() early" optimization to free up registers when we can, but as a side-effect it looked to the refcounter like the reference was done being used.

Not using it in this commit, just wanted to get the unmodified version in so it's easier to see the changes.

…ypes via their descrobject.c

This is for adding a guard on a non-immortal object, since we need to be safe against that object getting deallocated and a different object is allocated in its spot. We had support for this already, but it leaked memory. The biggest was that we never freed our runtimeICs, so if those ended up getting any GC references in them, then we would leak memory. So I started freeing those, but then that exposed the issue that the ICInvalidators expect that their dependent ICs never get freed. So I added back a mapping from ICSlotInfo-> ICInvalidators that reference them.

Create a simple Dockerfile

until I realize that it's because we were passing more tests than we expected.

The behavior changed in CPython 2.7.4, and Travis-CI runs 2.7.3.

Switch to CPython's descrobject.c

before we added a it as a module which made code fail which does something like __builtins__["unicode"]

Some packaging / distributing updates

Bump version numbers

update list of failing cpython tests

exec, input: if globals has no __builtins__ add it as a dictwrapper

since we were installing it from the LLVM APT repo, which they took down since it was getting too expensive. I guess we can just run with the gcc build until that situation gets resolved.

They end up generating "pass" statements with a lineno of 0, which trips an assert later on. This commit just sets them to have a lineno of 1. I'm not sure how to test this, since piping into stdin is supposed to be treated as a file (not as the repl). Though, we get that wrong right now.

Support empty lines on the repl

using cpython's `sys.flags` inplementation

enable `PyObject_Format` in `from_cpython/Objects/abstract.c`

Fix evaluation order for dict operations

+ use pyston::DenseMap to save a little more memory this saves about 5%-10% of peak memory on django

hidden classes: split into subclasses to reduce memory consumption

Update to fewer-pyston-changes virtualenv

and change it to use a unordered_map because it uses less memory in this case and is faster (I assume because it does not have align the key value tuples) saves about 10% of peak memory on django

ScopeNameUsage merge dicts into a single big one

Comment out some part of listobject.c, use the CPython list sort and apply some changes to existed Pyston code.

CPython listsort need "allocated" to check and throw exception: https://github.com/Daetalus/pyston/blob/d84105ffc8a4855bb6e00d9c22ba2baa8bddf969/from_cpython/Objects/listobject.c#L2091 https://github.com/Daetalus/pyston/blob/d84105ffc8a4855bb6e00d9c22ba2baa8bddf969/from_cpython/Objects/listobject.c#L2180

If BoxedList::allocated is -1, it means the items inside were changed. Some CPython list functions need this to check exceptions.

Switch to CPython list sort(Not its list implementation)

this saves a lot of memory

delete the llvm module after code generation

ICSlotInfo: remove old invalidator entries

undingen · 2016-08-07T13:20:49Z

src/runtime/set.cpp

+        PyErr_BadInternalCall();
+        return NULL;
+    }
+    return setPop(static_cast<BoxedSet*>(set));


this should be return callCXXFromStyle<CAPI>(setPop, static_cast<BoxedSet*>(set));

undingen and others added 30 commits May 24, 2016 16:59

Merge pull request pyston#1210 from undingen/str_tp_as_number

87f8c6a

str: use tp_as_sequence instead of tp_as_number

PyObject_New: register the type if the type is not yet registered

5e1a850

travis-ci: add some numpy requirements

b2a94fc

Create a simple Dockerfile

bf20355

that is just based off of pulling our latest release

Merge pull request pyston#1207 from undingen/register_types

61ac7b5

PyObject_New: register the type if the type is not yet registered

Fix a rewriter bug

a994ec0

We were doing a "call bumpUse() early" optimization to free up registers when we can, but as a side-effect it looked to the refcounter like the reference was done being used.

Copy in CPython's descrobject.c

8e8c9a8

Not using it in this commit, just wanted to get the unmodified version in so it's easier to see the changes.

Switch to using CPython's getset, member, wrapperdescr, and wrapper t…

4c3c693

…ypes via their descrobject.c

Reenable our tpp_call rewriting for these types

75d0050

Use CPython's PyMethodDescr_Type

0751c27

Reenable rewriting for method-descriptors

7f84725

Merge pull request pyston#1211 from kmod/docker2

c9cab0e

Create a simple Dockerfile

Get rid of the numpy patch

04f4515

cffi failures scare me

6dd62f7

until I realize that it's because we were passing more tests than we expected.

Manually specify the output of methoddescr.py

a11ffa7

The behavior changed in CPython 2.7.4, and Travis-CI runs 2.7.3.

Merge pull request pyston#1209 from kmod/cpython_descr2

7974eb8

Switch to CPython's descrobject.c

Some packaging / distributing updates

499c4c0

exec, input: if globals has no __builtins__ add it as a dictwrapper

f4f3d39

before we added a it as a module which made code fail which does something like __builtins__["unicode"]

Add a pyston/pyston-numpy docker image as well

865f7ba

Merge pull request pyston#1212 from kmod/packaging

f788cfa

Some packaging / distributing updates

update list of failing cpython tests

c4384c6

Bump version numbers

970ed60

Merge pull request pyston#1226 from kmod/packaging

c6d8021

Bump version numbers

Merge pull request pyston#1223 from undingen/cpython_tests2

08d75e5

update list of failing cpython tests

Merge pull request pyston#1221 from undingen/exec_builtins

e37660e

exec, input: if globals has no __builtins__ add it as a dictwrapper

Temporarily disable the clang build on travis-CI

d1b799e

since we were installing it from the LLVM APT repo, which they took down since it was getting too expensive. I guess we can just run with the gcc build until that situation gets resolved.

Fix gcc debug-mode issue

ecb7025

Boxiang Sun and others added 23 commits July 29, 2016 22:20

Copy listobject.c from CPython 2.7

d84105f

use PyObject_Format from abstract.c

6b0f785

using cpython's sys.flags inplementation

c7b8aa6

Merge pull request pyston#1313 from kmod/repl_empty

586a8c6

Support empty lines on the repl

Merge pull request pyston#1312 from aisk/sys_flags

10911d6

using cpython's `sys.flags` inplementation

fixes pyston#1191 evaluation order for dict operations.

7895da8

Merge pull request pyston#1311 from aisk/pyobject_format

84b0c7b

enable `PyObject_Format` in `from_cpython/Objects/abstract.c`

Merge pull request pyston#1302 from sizeoftank/literals_order_issue1191

747c0ad

Fix evaluation order for dict operations

hidden classes: split into subclasses to reduce memory consumption

cffc1b8

+ use pyston::DenseMap to save a little more memory this saves about 5%-10% of peak memory on django

Merge pull request pyston#1314 from undingen/hiddenclass_opt

2713ecf

hidden classes: split into subclasses to reduce memory consumption

Update to fewer-pyston-changes virtualenv

5d616f0

Merge pull request pyston#1315 from kmod/virtualenv

e21a0dc

Update to fewer-pyston-changes virtualenv

ScopeNameUsage merge dicts into a single big one

75f70a6

and change it to use a unordered_map because it uses less memory in this case and is faster (I assume because it does not have align the key value tuples) saves about 10% of peak memory on django

Merge pull request pyston#1316 from undingen/smaller_scopenameusage

6f34e11

ScopeNameUsage merge dicts into a single big one

Switch to CPython list sort

bd39c49

Comment out some part of listobject.c, use the CPython list sort and apply some changes to existed Pyston code.

Allow BoxedList::allocated is -1

d7993a9

If BoxedList::allocated is -1, it means the items inside were changed. Some CPython list functions need this to check exceptions.

Merge pull request pyston#1310 from Daetalus/list_cpython

fca39ba

Switch to CPython list sort(Not its list implementation)

delete the llvm module

d8f237b

this saves a lot of memory

ICSlotInfo: remove old invalidator entries

c7bd5c4

Merge pull request pyston#1317 from undingen/delete_llvm_mod

d817b29

delete the llvm module after code generation

Merge pull request pyston#1318 from undingen/slot_info_clear

81c744b

ICSlotInfo: remove old invalidator entries

Daetalus force-pushed the sklearn_nexedi_1 branch 2 times, most recently from 94530a6 to e1e9870 Compare August 6, 2016 18:26

test

9d6603e

Daetalus force-pushed the sklearn_nexedi_1 branch from e1e9870 to 9d6603e Compare August 6, 2016 22:14

undingen reviewed Aug 7, 2016
View reviewed changes

kmod force-pushed the master branch 2 times, most recently from 352fd89 to 6488a3e Compare October 28, 2020 21:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP]Scikit-learn commit #1308

[WIP]Scikit-learn commit #1308

Daetalus commented Jul 27, 2016 •

edited

Loading

undingen Aug 7, 2016 •

edited

Loading

[WIP]Scikit-learn commit #1308

Are you sure you want to change the base?

[WIP]Scikit-learn commit #1308

Conversation

Daetalus commented Jul 27, 2016 • edited Loading

undingen Aug 7, 2016 • edited Loading

Choose a reason for hiding this comment

Daetalus commented Jul 27, 2016 •

edited

Loading

undingen Aug 7, 2016 •

edited

Loading