Consider using lru_cache for the re.py caches #72380

Activity

1. rhettinger commented on Sep 17, 2016

   

   rhettinger

   on Sep 17, 2016

   ContributorAuthor

   More actions

   The last time we applied the LRU cache to the re.py module, the overhead of the pure python version resulted in a net performance decrease. But now we have a highly performance C version and should consider reinstating the code.
2. rhettinger

   added

   3.7 (EOL)end of lifeend of life

   on Sep 17, 2016
3. rhettinger

   assigned

   serhiy-storchaka

   on Sep 17, 2016
4. rhettinger

   added

   stdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directory

   performancePerformance or resource usagePerformance or resource usage

   on Sep 17, 2016
5. serhiy-storchaka commented on Sep 17, 2016

   

   serhiy-storchaka

   on Sep 17, 2016

   Member

   More actions

   Since that time the logic of re._compile() was changed. Now it can't just be wrapped with lru_cache().
6. serhiy-storchaka commented on Sep 18, 2016

   

   serhiy-storchaka

   on Sep 18, 2016

   Member

   More actions

   Added comments on Rietveld.
7. serhiy-storchaka commented on Sep 18, 2016

   

   serhiy-storchaka

   on Sep 18, 2016

   Member

   More actions

   lru_cache can be used for re._compile() if add the ability to bypass the cache and to validate cached value.
8. rhettinger commented on Sep 18, 2016

   

   rhettinger

   on Sep 18, 2016

   ContributorAuthor

   More actions

   Yes, I saw that. If a function could raise a NoCache exception, re._compile() could take advantage of it. But I don't feel good about going down that path (adding coupling between the caching decorator and the cached function). It would be better to keep the lru_cache API simple. I already made the mistake of expanding the API for typed=True just to accommodate a single use case (re.compile).
9. serhiy-storchaka commented on Sep 18, 2016

   

   serhiy-storchaka

   on Sep 18, 2016

   Member

   More actions

   Yes, raising an exception with a result as a payload is one option. Other option is to check a result. Something like:

   def _compile_is_valid(value):
       p, loc = value
       return loc is None or loc == _locale.setlocale(_locale.LC_CTYPE)
   
   def _compile_cache_if(value):
       p, loc = value
       return loc is not False
   
   @lru_cache(_MAXCACHE, is_valid=_compile_is_valid, cache_if=_compile_cache_if)
   def _compile1(pattern, flags):
       # internal: compile pattern
       if isinstance(pattern, _pattern_type):
           if flags:
               raise ValueError(
                   "cannot process flags argument with a compiled pattern")
           return pattern, False
       if not sre_compile.isstring(pattern):
           raise TypeError("first argument must be string or compiled pattern")
       p = sre_compile.compile(pattern, flags)
       if flags & DEBUG:
           return p, False
       if not (p.flags & LOCALE):
           return p, None
       if not _locale:
           return p, False
       return p, _locale.setlocale(_locale.LC_CTYPE)
   
   def _compile(pattern, flags):
       p, loc = _compile1(pattern, flags)
       return p
10. rhettinger commented on Sep 19, 2016

    

    rhettinger

    on Sep 19, 2016

    ContributorAuthor

    More actions

    I think I'll just take the low hanging fruit in _compile_repl and call it a day.
11. python-dev commented on Sep 19, 2016

    

    python-devmannequin

    on Sep 19, 2016

    Mannequin

    More actions

    New changeset 88110cfbf4dc by Raymond Hettinger in branch '3.6':
    Issue bpo-28193: Use lru_cache in the re module.
    https://hg.python.org/cpython/rev/88110cfbf4dc
12. rhettinger

    closed this as completedon Sep 19, 2016
13. ezio-melotti

    transferred this issue fromon Apr 10, 2022