Skip to content

Conversation

embray
Copy link
Contributor

@embray embray commented Apr 11, 2017

Arithmetic operations on subclasses of Proxy (specifically the C implementation) are slower (roughly three times) due to inexact match in the PyObject_Instance call in Proxy__WRAPPED_REPLACE_OR_RETURN_NULL. For simple C-level type compatibility checking PyObject_TypeCheck is much faster. In fact the PyObject_IsInstance is probably superfluous with this but I left it anyways.

Before:

In [1]: from lazy_object_proxy import Proxy

In [2]: a = Proxy(int)

In [3]: str(a)
Out[3]: '0'

In [4]: %timeit -n 1000000 -r 100 a + a
1000000 loops, best of 100: 61.1 ns per loop

In [5]: class MyProxy(Proxy): pass

In [6]: b = MyProxy(int)

In [7]: str(b)
Out[7]: '0'

In [8]: %timeit -n 1000000 -r 100 b + b
1000000 loops, best of 100: 171 ns per loop

After:

In [1]: from lazy_object_proxy import Proxy

In [2]: a = Proxy(int)

In [3]: str(a)
Out[3]: '0'

In [4]: %timeit -n 1000000 -r 100 a + a
1000000 loops, best of 100: 57.4 ns per loop

In [5]: class MyProxy(Proxy): pass

In [6]: b = MyProxy(int)

In [7]: str(b)
Out[7]: '0'

In [8]: %timeit -n 1000000 -r 100 b + b
1000000 loops, best of 100: 62.1 ns per loop

Speed up `Proxy__WRAPPED_REPLACE_OR_RETURN_NULL` for simple subclasses of `Proxy`
@ionelmc
Copy link
Owner

ionelmc commented Apr 11, 2017

Good catch! Now I'm thinking we don't need to do a full isinstance check (we don't care about things like https://docs.python.org/3/reference/datamodel.html?highlight=__instancecheck__#class.__instancecheck__) since a "fake instance" won't have the right memory layout anyway ...

@embray
Copy link
Contributor Author

embray commented Apr 11, 2017

I was thinking that too over lunch (it was a very lonely lunch :)

@embray
Copy link
Contributor Author

embray commented Apr 11, 2017

Would you like me to update it to just remove the full PyObject_IsInstance?

@ionelmc
Copy link
Owner

ionelmc commented Apr 11, 2017

Yes, lets try it.

@ionelmc
Copy link
Owner

ionelmc commented Apr 11, 2017

Also, update CHANGELOG/AUTHORS.rst.

embray added 2 commits April 11, 2017 16:34
…heck since only actual subtypes will be compatible with this code anyways
@embray
Copy link
Contributor Author

embray commented Apr 11, 2017

Done--I agree it makes sense to submit the change upstream to wrapt as well.

@codecov-io
Copy link

Codecov Report

Merging #18 into master will decrease coverage by 0.59%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #18     +/-   ##
=========================================
- Coverage   93.81%   93.22%   -0.6%     
=========================================
  Files           8        7      -1     
  Lines        2038     1594    -444     
  Branches      245      245             
=========================================
- Hits         1912     1486    -426     
+ Misses        101       83     -18     
  Partials       25       25
Impacted Files Coverage Δ
src/lazy_object_proxy/__init__.py 85.71% <0%> (-14.29%) ⬇️
tests/test_lazy_object_proxy.py 92.11% <0%> (-0.44%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7033131...cb8ad7b. Read the comment docs.

@ionelmc ionelmc merged commit cb8ad7b into ionelmc:master Apr 13, 2017
@ionelmc
Copy link
Owner

ionelmc commented Apr 13, 2017

Allright, thanks!

@embray embray deleted the patch-1 branch April 14, 2017 09:14
@embray
Copy link
Contributor Author

embray commented Apr 14, 2017

Thanks to you too!

@ionelmc
Copy link
Owner

ionelmc commented May 4, 2017

Published 1.3.0 on PyPI.

@embray embray restored the patch-1 branch January 30, 2020 11:39
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.

3 participants