Skip to content

Document that lru_cache uses hard references #88476

Closed

Description

@wouterdb
mannequin
BPO 44310
Nosy @rhettinger, @serhiy-storchaka, @cryvate, @pablogsal, @miss-islington, @nanjekyejoannah, @@wouterdb, @Fidget-Spinner
PRs
  • bpo-44310: added docs to functools to warn for memory leaks #26528
  • bpo-44310: Note that lru_cache keep references to both arguments and results #26715
  • [3.10] bpo-44310: Note that lru_cache keep references to both arguments and results (GH-26715) #26716
  • bpo-44310: Add a FAQ entry for caching method calls #26731
  • [3.10] bpo-44310: Add a FAQ entry for caching method calls (GH-26731) #26777
  • [3.9] bpo-44310: Add a FAQ entry for caching method calls (GH-26731) #26778
  • bpo-44310: Remove dubious suggestion in cache example #26789
  • Superseder
  • bpo-19859: functools.lru_cache keeps objects alive forever
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/rhettinger'
    closed_at = <Date 2021-06-14.05:54:45.410>
    created_at = <Date 2021-06-04.11:45:58.109>
    labels = ['3.11', 'library', '3.9', '3.10', 'performance']
    title = 'Document that lru_cache uses hard references'
    updated_at = <Date 2021-06-20.15:11:01.892>
    user = 'https://github.com/wouterdb'

    bugs.python.org fields:

    activity = <Date 2021-06-20.15:11:01.892>
    actor = 'rhettinger'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2021-06-14.05:54:45.410>
    closer = 'rhettinger'
    components = ['Library (Lib)']
    creation = <Date 2021-06-04.11:45:58.109>
    creator = 'Wouter De Borger2'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 44310
    keywords = ['patch']
    message_count = 21.0
    messages = ['395075', '395087', '395088', '395125', '395144', '395149', '395152', '395157', '395158', '395336', '395774', '395796', '395887', '395912', '396016', '396017', '396018', '396019', '396020', '396021', '396178']
    nosy_count = 9.0
    nosy_names = ['rhettinger', 'python-dev', 'serhiy.storchaka', 'cryvate', 'pablogsal', 'miss-islington', 'nanjekyejoannah', 'Wouter De Borger2', 'kj']
    pr_nums = ['26528', '26715', '26716', '26731', '26777', '26778', '26789']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = '19859'
    type = 'resource usage'
    url = 'https://bugs.python.org/issue44310'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    Activity

    1. wouterdb commented on Jun 4, 2021

      wouterdbmannequin
      MannequinAuthor

      # Problem

      the functools.lru_cache decorator locks all arguments to the function in memory (inclusing self), causing hard to find memory leaks.

      # Expected

      I had assumed that the lru_cache would keep weak-references and that when an object is garbage colected, all its cache entries expire as unreachable. This is not the case.

      # Solutions

      1. I think it is worth at least mentioning this behavior in de documentation.
      2. I also think it would be good if the LRU cache actually uses weak references.

      I will try to make a PR for this.

    2. added
      3.11only security fixes
      stdlibStandard Library Python modules in the Lib/ directory
      performancePerformance or resource usage
      on Jun 4, 2021
    3. 16 remaining items

    4. Cryvate commented on Jun 8, 2021

      Cryvatemannequin
      Mannequin

      Reading this bug thread last week made me realize we had made the following error in our code:

      class SomethingView():
          @functools.lru_cache()
          def get_object(self):
              return self._object

      Now, as this class was instantiated for every (particular kind of) request to a webserver and this method called (a few times), the lru_cache just kept filling up and up. We had been having a memory leak we couldn't track down, and this was it.

      I think this is an easy mistake to make and it was rooted, not so much in hard references though (without that though, it would have not leaked memory) but because of the fact the cache lives on the class and not the object.

    5. rhettinger commented on Jun 14, 2021

      @rhettinger
      Contributor

      New changeset fafcfff by Raymond Hettinger in branch 'main':
      bpo-44310: Note that lru_cache keep references to both arguments and results (GH-26715)
      fafcfff

    6. rhettinger commented on Jun 14, 2021

      @rhettinger
      Contributor

      New changeset 809c3fa by Miss Islington (bot) in branch '3.10':
      bpo-44310: Note that lru_cache keep references to both arguments and results (GH-26715) (GH-26716)
      809c3fa

    7. rhettinger commented on Jun 15, 2021

      @rhettinger
      Contributor

      See PR 26731 for a draft FAQ entry. Let me know what you think.

    8. Cryvate commented on Jun 16, 2021

      Cryvatemannequin
      Mannequin

      PR 26731 looks very good to me. My only comment, which I am not sure is worthy of adding/is a general lru_cache thing, that "instances
      are kept alive until they age out of the cache or until the cache is
      cleared", if you are creating instances and calling this method all the time, will lead to an infinite memory leak.

      Not sure whether that's too specific to the problem we encountered and we are all consenting adults and should infer this or it is helpful: leave it up to your/other people's judgement.

      P.S. In the programming.rst there is also the "Why are default values shared between objects?" section which actually uses default values to make its own poor version of a cache. It should probably at least mention lru_cache could be used (unless you particularly need callers to be able to pass their own cache).

    9. rhettinger commented on Jun 17, 2021

      @rhettinger
      Contributor

      if you are creating instances and calling this method
      all the time, will lead to an infinite memory leak.

      Your words aren't making any sense to me. The default
      lru_cache will never hold more than maxsize entries.
      The default maxsize is 128. How is that "infinite"?

    10. Cryvate commented on Jun 17, 2021

      Cryvatemannequin
      Mannequin

      I clearly was missing some words there Raymond. I meant, if one has set maxsize=None.

    11. Cryvate commented on Jun 17, 2021

      Cryvatemannequin
      Mannequin

      (but consenting adults, setting max_size=None for "efficiency", you better be sure what you are doing in a long running process and making sure it cannot grow unbounded.)

    12. rhettinger commented on Jun 17, 2021

      @rhettinger
      Contributor

      I meant, if one has set maxsize=None.

      The docs already say, "If maxsize is set to None, the LRU feature is disabled and the cache can grow without bound."

    13. rhettinger commented on Jun 17, 2021

      @rhettinger
      Contributor

      New changeset 7f01f77 by Raymond Hettinger in branch 'main':
      bpo-44310: Add a FAQ entry for caching method calls (GH-26731)
      7f01f77

    14. rhettinger commented on Jun 17, 2021

      @rhettinger
      Contributor

      New changeset 77eaf14 by Miss Islington (bot) in branch '3.10':
      bpo-44310: Add a FAQ entry for caching method calls (GH-26731) (GH-26777)
      77eaf14

    15. rhettinger commented on Jun 20, 2021

      @rhettinger
      Contributor

      Adding a weak referencing recipe here just so I can find it in the future.

      --------------------------------------------------------------------------

      import functools
      import weakref
      
      def weak_lru(maxsize=128, typed=False):
          """LRU Cache decorator that keeps a weak reference to "self".
      Only provides benefit if the instances are so large that
      it is impractical to wait for them to age out of the cache.
      
      When the instance is freed, the cache entry still remains
      but will be unreachable.
      
      If new instances will be created that are equal to the ones
      retired by the weak reference, we lose all the benefits of
      having cached the previous call.  
      
      If the class defines __slots__, be sure to add '__weakref__'
      to make the instances weak referenceable.
      
      """
      
          def decorator(func):
      
              ref = weakref.ref
      
              @functools.lru_cache(maxsize, typed)
              def _func(_self, /, *args, **kwargs):
                  return func(_self(), *args, **kwargs)
      
              @functools.wraps(func)
              def wrapper(self, /, *args, **kwargs):
                  return _func(ref(self), *args, **kwargs)
      
              return wrapper
      
          return decorator
    16. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    3.10 (EOL)end of life3.11only security fixes3.9 (EOL)end of lifeperformancePerformance or resource usagestdlibStandard Library Python modules in the Lib/ directory

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions