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.

Sign up for GitHub

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

Jump to bottom

Can't use both weakref's & cache #8825

Closed
skirpichev opened this issue Jan 15, 2015 · 11 comments
Closed

Can't use both weakref's & cache #8825

skirpichev opened this issue Jan 15, 2015 · 11 comments
Labels
core.cache

Comments

@skirpichev
Copy link
Contributor

Without the cache:

$ ./bin/isympy -q -C

Welcome to pylab, a matplotlib-based Python environment [backend: TkAgg].
For more information, type 'help(pylab)'.
IPython console for SymPy 0.7.6-git (Python 2.7.3-64-bit) (ground types: gmpy, cache: off)

In [1]: import weakref

In [2]: d = weakref.WeakKeyDictionary([(x,1), (y, 2)])

In [3]: print(d.items())
[(x, 1), (y, 2)]

In [4]: del x

In [5]: print(d.items())
[(y, 2)]

With (builtin version, no fastcache):

...
n [4]: del x

In [5]: print(d.items())
[(x, 1), (y, 2)]

clear_cache() doesn't help here.

BTW, I use this trivial patch to make Symbol's weakref-able:

diff --git a/sympy/core/symbol.py b/sympy/core/symbol.py
index ebd3diff --git a/sympy/core/symbol.py b/sympy/core/symbol.py
index ebd3a52..0e8c1d2 100644
--- a/sympy/core/symbol.py
+++ b/sympy/core/symbol.py
@@ -35,7 +35,7 @@ class Symbol(AtomicExpr, Boolean):

     is_comparable = False

-    __slots__ = ['name']
+    __slots__ = ['name', '__weakref__']

     is_Symbol = True
@skirpichev
Copy link
Contributor Author

@pbrady, what do you think?

@pbrady
Copy link
Member

pbrady commented Jan 15, 2015

What happens if you manually trigger gc after clearing the cache?

@skirpichev
Copy link
Contributor Author

On Thu, Jan 15, 2015 at 05:46:23AM -0800, Peter Brady wrote:

What happens if you manually trigger gc after clearing the cache?

Nothing. Perhaps, I do something wrong?

$ ./bin/isympy -q

Welcome to pylab, a matplotlib-based Python environment [backend: TkAgg].
For more information, type 'help(pylab)'.
IPython console for SymPy 0.7.6-git (Python 2.7.3-64-bit) (ground types: gmpy)

In [1]: import weakref, gc

In [2]: d = weakref.WeakKeyDictionary([(x,1), (y, 2)])

In [3]: from sympy.core.cache import clear_cache

In [4]: clear_cache()

In [5]: del x

In [6]: clear_cache()

In [7]: gc.collect()
Out[7]: 1766

In [8]: d.items()
Out[8]: [(x, 1), (y, 2)]

In [9]: gc.garbage
Out[9]: []

@pbrady
Copy link
Member

pbrady commented Jan 15, 2015

Nothing. Perhaps, I do something wrong?

I don't think so. This doesn't make a lot of sense though. After a call to clear_cache, there shouldn't be any more instances around. Maybe clear_cache is broken?

@skirpichev
Copy link
Contributor Author

On Thu, Jan 15, 2015 at 08:00:26AM -0800, Peter Brady wrote:

 Nothing. Perhaps, I do something wrong?

I don't think so. This doesn't make a lot of sense though. After a call to
clear_cache, there shouldn't be any more instances around. Maybe
clear_cache is broken?

AFAIR, I tried also del'ing everything from the CACHE.

@pbrady
Copy link
Member

pbrady commented Jan 15, 2015

I thought maybe IPython was keeping extra references so I tried in the repl. Same results. Cache is definitely empty.

>>> from sympy import symbols
>>> from weakref import WeakKeyDictionary
>>> x, y = symbols('x y')
>>> from sympy.core.cache import clear_cache
>>> d = WeakKeyDictionary([(x, 1), (y, 2)])
>>> clear_cache()
>>> del x
>>> list(d.items())
[(x, 1), (y, 2)]
>>> clear_cache()
>>> del x
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'x' is not defined
>>> list(d.items())
[(x, 1), (y, 2)]
>>> import gc
>>> gc.collect()
0
>>> from sympy.core.cache import print_cache
>>> print_cache()
sort_key CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
_subs CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
has CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
sort_key CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
sort_key CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
as_leading_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
expand CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
__new__ CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
args CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
as_two_terms CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
as_coeff_mul CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
_eval_derivative CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
sort_key CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
__new__ CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
as_coeff_add CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
_eval_derivative CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
as_two_terms CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
extract_leading_order CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
__new__ CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
__new__ CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
args CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
args CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
__new_stage2__ CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
sort_key CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
sort_key CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
__xnew__ CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
__new__ CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
_taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
sort_key CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
sort_key CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
_eval CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
_nP CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
_AOP_product CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
_stirling1 CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
_stirling2 CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
_nT CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
taylor_term CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
__trigsimp CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
__new__ CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
contains CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
sign CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
limitinf CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
mrv_leadterm CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
_roots_trivial CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
make_wilds CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
sincos_pattern CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
tansec_pattern CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
cotcsc_pattern CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
heaviside_pattern CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
_pat_sincos CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)
_rewrite_single CacheInfo(hits=0, misses=0, maxsize=1000, currsize=0)

@skirpichev
Copy link
Contributor Author

Perhaps, this is the solution:
https://bitbucket.org/thesheep/cpython-lru_cache-weakref/commits/66c1c9f3256785552224ca177ed77a8312de6bb8

@pbrady
Copy link
Member

pbrady commented Mar 8, 2015

I wasn't aware that this option was added. We should benchmark this (pure python cache) and see what the improvements are. My hunch is that it will be significantly slower when one already specifies a bounded cache but it may prevent the unbounded cached from blowing up.

@skirpichev
Copy link
Contributor Author

I wasn't aware that this option was added.

No, it's non-upstream patch. But I think that the problem is same.

@skirpichev
Copy link
Contributor Author

This is valid for fastcache too.

$ git diff
diff --git a/sympy/core/symbol.py b/sympy/core/symbol.py
index 3816cff..df7e71d 100644
--- a/sympy/core/symbol.py
+++ b/sympy/core/symbol.py
@@ -34,7 +34,7 @@ class Symbol(AtomicExpr, Boolean):

     is_comparable = False

-    __slots__ = ['name']
+    __slots__ = ['name', '__weakref__']

     is_Symbol = True

$ cat a.py
from sympy import *
x, y = symbols('x, y')
import weakref
d = weakref.WeakKeyDictionary([(x,1), (y, 2)])
print(d.items())
del x
print(d.items())
$ SYMPY_USE_CACHE=no python a.py
[(x, 1), (y, 2)]
[(y, 2)]
$ SYMPY_USE_CACHE=yes python a.py
[(x, 1), (y, 2)]
[(x, 1), (y, 2)]

skirpichev added a commit to diofant/diofant that referenced this issue May 2, 2015
@skirpichev
Add weakref support for Symbols, fix sympy/sympy#8825
32704bf 
At this commit::

    $ ./bin/isympy -q
    Python console for SymPy 0.7.6-git (Python 2.7.3-64-bit) (ground types: python)

    >>> import weakref
    >>> d = weakref.WeakKeyDictionary([(x,1), (y, 2)])
    >>> print(d.items())
    [(x, 1), (y, 2)]
    >>> del x
    >>> print(d.items())
    [(y, 2)]
skirpichev added a commit to diofant/diofant that referenced this issue Jan 17, 2016
@skirpichev
Add weakref support for Basic's
e6d4930 
This fixes sympy/sympy#8825:

    >>> import weakref
    >>> import sympy
    >>> x, y = sympy.Symbol('x'), sympy.Symbol('y')
    >>> d = weakref.WeakKeyDictionary([(x, 1), (y, 2)])
    >>> print(list(d.items()))
    [(y, 2), (x, 1)]
    >>> del x
    >>> print(list(d.items()))
    [(y, 2)]

Cache now uses WeakValueDictionary.
skirpichev added a commit to diofant/diofant that referenced this issue Jan 18, 2016
@skirpichev
Add weakref support for Basic's
a8cebc8 
This fixes sympy/sympy#8825:

    >>> import weakref
    >>> import sympy
    >>> x, y = sympy.Symbol('x'), sympy.Symbol('y')
    >>> d = weakref.WeakKeyDictionary([(x, 1), (y, 2)])
    >>> print(list(d.items()))
    [(y, 2), (x, 1)]
    >>> del x
    >>> print(list(d.items()))
    [(y, 2)]

Cache now uses WeakValueDictionary.
skirpichev added a commit to diofant/diofant that referenced this issue Jan 25, 2016
@skirpichev
Add weakref support for Basic's
4175fad 
This fixes sympy/sympy#8825:

    >>> import weakref
    >>> import sympy
    >>> x, y = sympy.Symbol('x'), sympy.Symbol('y')
    >>> d = weakref.WeakKeyDictionary([(x, 1), (y, 2)])
    >>> print(list(d.items()))
    [(y, 2), (x, 1)]
    >>> del x
    >>> print(list(d.items()))
    [(y, 2)]

Cache now uses WeakValueDictionary.
skirpichev added a commit to diofant/diofant that referenced this issue Feb 2, 2016
@skirpichev
Add weakref support for Basic's
8262da0 
This fixes sympy/sympy#8825:

    >>> import weakref
    >>> import sympy
    >>> x, y = sympy.Symbol('x'), sympy.Symbol('y')
    >>> d = weakref.WeakKeyDictionary([(x, 1), (y, 2)])
    >>> print(list(d.items()))
    [(y, 2), (x, 1)]
    >>> del x
    >>> print(list(d.items()))
    [(y, 2)]

Cache now uses WeakValueDictionary.
skirpichev added a commit to diofant/diofant that referenced this issue Feb 22, 2016
@skirpichev
Add weakref support for Basic's
4c6ee38 
This fixes sympy/sympy#8825:

    >>> import weakref
    >>> import sympy
    >>> x, y = sympy.Symbol('x'), sympy.Symbol('y')
    >>> d = weakref.WeakKeyDictionary([(x, 1), (y, 2)])
    >>> print(list(d.items()))
    [(y, 2), (x, 1)]
    >>> del x
    >>> print(list(d.items()))
    [(y, 2)]

Cache now uses WeakValueDictionary.
skirpichev added a commit to skirpichev/diofant that referenced this issue Nov 2, 2016
@skirpichev
Add regression tests & mention closed issues
74ade76 
    close sympy/sympy#3112 (MrvAsympt was added in diofant#6)
    close sympy/sympy#9173 (test was added in 5a510ac)
    close sympy/sympy#9808 (fixed in 09e539b)
    close sympy/sympy#9341 (fixed in af98a00)
    close sympy/sympy#9908 (fixed in cc3fa8d)
    close sympy/sympy#6171 (test added in d278031)
    close sympy/sympy#9276 (diagnose_imports.py removed in ab8c535)
    close sympy/sympy#10201 (fixed in 0d0fc5f)
    close sympy/sympy#9057 (test was added in 8290a0c)
    close sympy/sympy#11159 (test was added in ffb76cb)
    close sympy/sympy#2839 (new AST transformers are used, see diofant#278 and diofant#167)
    close sympy/sympy#11081 (see ed01e16 and bb92329)
    close sympy/sympy#10974 (see 73fc425)
    close sympy/sympy#10806 (test in 539929a)
    close sympy/sympy#10801 (test in 2fe3da5)
    close sympy/sympy#9549 (test in 88bdefa)
    close sympy/sympy#4231 (test was added in fb411d5)
    close sympy/sympy#8634 (see 2fcbb58)
    close sympy/sympy#8481 (see 1ef20d3)
    close sympy/sympy#9956 (fixed in a34735f)
    close sympy/sympy#9747 (see e117c60)
    close sympy/sympy#7853 (see 3e4fbed)
    close sympy/sympy#9634 (see 2be03f5)
    close sympy/sympy#8500 (fixed in diofant#104 and finally in diofant#316)
    close sympy/sympy#9192 (see 9bf622f)
    close sympy/sympy#7130 (see e068fa3)
    close sympy/sympy#8514 (see b2d543b)
    close sympy/sympy#9334 (see 90de625)
    close sympy/sympy#8229 (see 9755b89)
    close sympy/sympy#8061 (see 7054f06)
    close sympy/sympy#7872 (tested in diofant#6)
    close sympy/sympy#3496 (tested in test_log_symbolic)
    close sympy/sympy#2929 (see da7db7a)
    close sympy/sympy#8203 (oo is not a real, see diofant#36)
    close sympy/sympy#7649 (0 is imaginary since diofant#8)
    close sympy/sympy#7256 (fixed in c0a4549)
    close sympy/sympy#6783 (see cb28d63)
    close sympy/sympy#5662 (is_integer issue fixed in 6bfa9f8, there is no is_bounded anymore)
    close sympy/sympy#5295 (fixed with diofant#354)
    close sympy/sympy#4856 (we now have flake/pep tests)
    close sympy/sympy#4555 (flake8 enabled after diofant#214)
    close sympy/sympy#5773 (cmp_to_key removed after diofant#164 and c9acbf0)
    close sympy/sympy#5484 (see above)

    Added regression tests:
    from https://groups.google.com/forum/#!topic/sympy/LkTMQKC_BOw
    fixes sympy/sympy#8825 (probably via diofant#209)
    fixes sympy/sympy#8635
    fixes sympy/sympy#8157
    fixes sympy/sympy#7872
    fixes sympy/sympy#7599
    fixes sympy/sympy#6179
    fixes sympy/sympy#5415
    fixes sympy/sympy#2865
    fixes sympy/sympy#5907
    fixes sympy/sympy#11722

    Closes diofant#347
skirpichev added a commit to skirpichev/diofant that referenced this issue Nov 2, 2016
@skirpichev
Add regression tests & mention closed issues
48c035e 
    close sympy/sympy#3112 (MrvAsympt was added in diofant#6)
    close sympy/sympy#9173 (test was added in 5a510ac)
    close sympy/sympy#9808 (fixed in 09e539b)
    close sympy/sympy#9341 (fixed in af98a00)
    close sympy/sympy#9908 (fixed in cc3fa8d)
    close sympy/sympy#6171 (test added in d278031)
    close sympy/sympy#9276 (diagnose_imports.py removed in ab8c535)
    close sympy/sympy#10201 (fixed in 0d0fc5f)
    close sympy/sympy#9057 (test was added in 8290a0c)
    close sympy/sympy#11159 (test was added in ffb76cb)
    close sympy/sympy#2839 (new AST transformers are used, see diofant#278 and diofant#167)
    close sympy/sympy#11081 (see ed01e16 and bb92329)
    close sympy/sympy#10974 (see 73fc425)
    close sympy/sympy#10806 (test in 539929a)
    close sympy/sympy#10801 (test in 2fe3da5)
    close sympy/sympy#9549 (test in 88bdefa)
    close sympy/sympy#4231 (test was added in fb411d5)
    close sympy/sympy#8634 (see 2fcbb58)
    close sympy/sympy#8481 (see 1ef20d3)
    close sympy/sympy#9956 (fixed in a34735f)
    close sympy/sympy#9747 (see e117c60)
    close sympy/sympy#7853 (see 3e4fbed)
    close sympy/sympy#9634 (see 2be03f5)
    close sympy/sympy#8500 (fixed in diofant#104 and finally in diofant#316)
    close sympy/sympy#9192 (see 9bf622f)
    close sympy/sympy#7130 (see e068fa3)
    close sympy/sympy#8514 (see b2d543b)
    close sympy/sympy#9334 (see 90de625)
    close sympy/sympy#8229 (see 9755b89)
    close sympy/sympy#8061 (see 7054f06)
    close sympy/sympy#7872 (tested in diofant#6)
    close sympy/sympy#3496 (tested in test_log_symbolic)
    close sympy/sympy#2929 (see da7db7a)
    close sympy/sympy#8203 (oo is not a real, see diofant#36)
    close sympy/sympy#7649 (0 is imaginary since diofant#8)
    close sympy/sympy#7256 (fixed in c0a4549)
    close sympy/sympy#6783 (see cb28d63)
    close sympy/sympy#5662 (is_integer issue fixed in 6bfa9f8, there is no is_bounded anymore)
    close sympy/sympy#5295 (fixed with diofant#354)
    close sympy/sympy#4856 (we now have flake/pep tests)
    close sympy/sympy#4555 (flake8 enabled after diofant#214)
    close sympy/sympy#5773 (cmp_to_key removed after diofant#164 and c9acbf0)
    close sympy/sympy#5484 (see above)

    Added regression tests:
    from https://groups.google.com/forum/#!topic/sympy/LkTMQKC_BOw
    fixes sympy/sympy#8825 (probably via diofant#209)
    fixes sympy/sympy#8635
    fixes sympy/sympy#8157
    fixes sympy/sympy#7872
    fixes sympy/sympy#7599
    fixes sympy/sympy#6179
    fixes sympy/sympy#5415
    fixes sympy/sympy#2865
    fixes sympy/sympy#5907
    fixes sympy/sympy#11722

    Closes diofant#347
@skirpichev
Copy link
Contributor Author

No activity. This is fixed in the diofant, thus not a problem for me. Closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
core.cache
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use WeakValueDictionary in cache diofant/diofant
3 participants
@skirpichev @debugger22 @pbrady