Skip to content

Commit

Permalink
Fix a bug in OPT-12 causing incorrect caching of a method address for…
Browse files Browse the repository at this point in the history
… a global type subclass (#462)

* Add sqlalchemy tests to better understand reported bug

* Tune warnings

* Try bumping load known methods

* Roll that opt back

* Dont optimize methods for type subclasses

* Update release notes
  • Loading branch information
tonybaloney authored Dec 9, 2021
1 parent 774b736 commit 93d0447
Show file tree
Hide file tree
Showing 10 changed files with 58 additions and 6 deletions.
9 changes: 9 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,12 @@ src/pyjion/*.so
src/pyjion/*.pyd
*.py[co]
__pycache__

Tests/logs/
pyjion-lib/src/packages/
pyjion-lib/bin/
pyjion-lib/src/Pyjionlib/
pyjion-lib/src/.vs/
pyjion-lib/src/Pyjionlib.Test/
Tests/benchmarks/profiles/
wheelhouse/
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Release notes

## 1.2.1

* Fixes a bug in OPT-12 (method caching) where it would store the incorrect cache address for a global type of subclass. Bug seen in SQLalchemy.

## 1.2.0

* PGC unboxing errors are avoided when functions are called with different argument types to those it was optimized with.
Expand Down
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,9 @@ if(NOT WIN32)
add_definitions(-DTARGET_UNIX)
message(STATUS "Enabling UNIX Patches")
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
add_compile_options(-Wno-null-arithmetic -Wno-switch)
add_compile_options(-Wno-null-arithmetic -Wno-switch -Wno-pragma-pack)
else(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
add_compile_options(-Wno-conversion-null -Wno-pointer-arith -Wno-pragma-pack -Wno-switch)
add_compile_options(-Wno-conversion-null -Wno-pointer-arith -Wno-switch -Wno-missing-profile)
endif(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
else()
add_definitions(-DWINDOWS=1)
Expand Down
3 changes: 2 additions & 1 deletion Tests/requirements_test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ rich
pyyaml
numpy
pytest>=6.2.5
pandas
pandas
sqlalchemy
File renamed without changes.
1 change: 0 additions & 1 deletion Tests/test_pyyaml.py → Tests/thirdparty/test_pyyaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
has_pyyaml = False


@pytest.mark.xfail(reason="optimization bug, see #309")
@pytest.mark.skipif(not has_pyyaml, reason="No pyyaml installed")
@pytest.mark.external
def test_load_yaml():
Expand Down
34 changes: 34 additions & 0 deletions Tests/thirdparty/test_sqlalchemy.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import pytest
try:
from sqlalchemy import Column, select, Integer, String, create_engine
from sqlalchemy.orm import declarative_base, Session

has_lib = True
except ImportError:
has_lib = False


@pytest.mark.graph
@pytest.mark.skipif(not has_lib, reason="Missing library")
@pytest.mark.external
def test_base_type():
# declarative base class
Base = declarative_base()
engine = create_engine('sqlite://')

class Post(Base):
__tablename__ = 'post'
id = Column(Integer, autoincrement=True, primary_key=True)
name = Column(String)

# create session and add objects
with Session(engine) as session:
Base.metadata.create_all(engine)

session.add(Post(name='first post'))
session.add(Post(name='second post'))
session.commit()

sp = select(Post)
ps = session.execute(select(Post)).scalars().all()
assert len(ps) == 2
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

setup(
name='pyjion',
version='1.2.0',
version='1.2.1',
description='A JIT compiler wrapper for CPython',
author='Anthony Shaw',
author_email='[email protected]',
Expand Down
2 changes: 1 addition & 1 deletion src/pyjion/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from enum import IntFlag, IntEnum
from dataclasses import dataclass

__version__ = '1.2.0'
__version__ = '1.2.1'


def _no_dotnet(path):
Expand Down
5 changes: 5 additions & 0 deletions src/pyjion/pycomp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2151,6 +2151,11 @@ void PythonCompiler::emit_builtin_method(PyObject* name, AbstractValue* typeValu
return;
}

if (PyType_HasFeature(pyType, Py_TPFLAGS_TYPE_SUBCLASS)){
emit_load_method(name);// Can't inline this type of method
return;
}

auto meth = _PyType_Lookup(pyType, name);

if (meth == nullptr || !PyType_HasFeature(Py_TYPE(meth), Py_TPFLAGS_METHOD_DESCRIPTOR)) {
Expand Down

0 comments on commit 93d0447

Please sign in to comment.