[issue28293] Don't completely dump the regex cache when full

Serhiy Storchaka added the comment: Thank you for your reviews. Currently the caching logic is simpler, it no longer depends on the LOCALE flag and the current locale. But it still is too complex for lru_cache. It needs a way to bypass caching based on arguments or result. --

[issue28293] Don't completely dump the regex cache when full

Serhiy Storchaka added the comment: New changeset 114454e9f6addbcb364e9a37102c8131ae2da1dd by Serhiy Storchaka in branch 'master': bpo-28293: Don't completely dump the regex cache when full. (#3768) https://github.com/python/cpython/commit/114454e9f6addbcb364e9a37102c8131ae2da1dd --

[issue28293] Don't completely dump the regex cache when full

Changes by Serhiy Storchaka : -- nosy: +barry ___ Python tracker ___ ___

[issue28293] Don't completely dump the regex cache when full

Changes by Serhiy Storchaka : -- pull_requests: +3753 ___ Python tracker ___

[issue28293] Don't completely dump the regex cache when full

INADA Naoki added the comment: LGTM for re_cache_ordered_dict_popitem.patch -- nosy: +inada.naoki ___ Python tracker ___

[issue28293] Don't completely dump the regex cache when full

Changes by Serhiy Storchaka : -- versions: -Python 3.6 ___ Python tracker ___

[issue28293] Don't completely dump the regex cache when full

STINNER Victor added the comment: re_cache_ordered_dict_popitem.patch: LGTM, it can be pushed to the default branch. -- "This is unlikely possible with current API of lru_cache. Testing flags before looking up in a cache adds an overhead." Oh wait, I looked at the code and this case is

[issue28293] Don't completely dump the regex cache when full

Serhiy Storchaka added the comment: This is unlikely possible with current API of lru_cache. Testing flags before looking up in a cache adds an overhead. -- ___ Python tracker

[issue28293] Don't completely dump the regex cache when full

STINNER Victor added the comment: Can't we redesign the function to reuse @functools.lru_cache(_MAXCACHE) but take care of the debug flag? For example, split the function in smaller parts. -- nosy: +haypo ___ Python tracker

[issue28293] Don't completely dump the regex cache when full

Changes by Raymond Hettinger : -- assignee: rhettinger -> serhiy.storchaka ___ Python tracker ___

[issue28293] Don't completely dump the regex cache when full

Changes by Serhiy Storchaka : -- components: +Regular Expressions nosy: +ezio.melotti, mrabarnett ___ Python tracker ___

[issue28293] Don't completely dump the regex cache when full

Serhiy Storchaka added the comment: The re module now depends on the enum module, and the latter already depends on OrderedDict. Thus the patch doesn't introduce new dependency. Python implementation of OrderedDict doesn't override __getitem__ and therefore don't slow down the common case. I

[issue28293] Don't completely dump the regex cache when full

Raymond Hettinger added the comment: I would rather not introduce an OrderedDict dependency to the re module while other implementations are using the pure python version or where the OD implementation is in flux. This is a barely worth it proposal, so we should either make a modest,

[issue28293] Don't completely dump the regex cache when full

Serhiy Storchaka added the comment: It is applied cleanly on the default branch. I don't think there is a need to open a new issue. The last patch implements the original Raymond's intention, but taking into account all comments: drops the oldest item, doesn't depend on implementation details

[issue28293] Don't completely dump the regex cache when full

SilentGhost added the comment: Serhiy, your patch doesn't seem to apply cleanly. Also, it's perhaps worth opening the issue if you'd like to continue working on it. -- nosy: +SilentGhost ___ Python tracker

[issue28293] Don't completely dump the regex cache when full

Serhiy Storchaka added the comment: > Why not OrderedDict.popitem(last=False)? Yes, this should work. Here is a patch. -- Added file: http://bugs.python.org/file44855/re_cache_ordered_dict_popitem.patch ___ Python tracker

[issue28293] Don't completely dump the regex cache when full

Raymond Hettinger added the comment: This has gotten crazy. I withdraw the suggestion. -- resolution: -> rejected status: open -> closed ___ Python tracker

[issue28293] Don't completely dump the regex cache when full

Xiang Zhang added the comment: Why not OrderedDict.popitem(last=False)? -- ___ Python tracker ___ ___

[issue28293] Don't completely dump the regex cache when full

Serhiy Storchaka added the comment: > Maybe use OrderedDict? But this adds heavy dependency to the re module. We can use the same trick as in the enum module. Here is a patch. -- Added file: http://bugs.python.org/file44854/re_cache_ordered_dict_del_first.patch

[issue28293] Don't completely dump the regex cache when full

Serhiy Storchaka added the comment: > Perhaps: _cache.pop(next(iter(_cache))) This can raise KeyError if the cache is cleared in other thread. And it is a little slower. I remember why I didn't propose this idea earlier. This depends on the ordering of dict. But this is implementation

[issue28293] Don't completely dump the regex cache when full

Raymond Hettinger added the comment: Perhaps: _cache.pop(next(iter(_cache))) The for-loop version indirect about what it is trying to do and relies on an obscure quirk of exactly when it is an error to mutate while iterating. I do like that the side-effect of the compact dict is that is lets

[issue28293] Don't completely dump the regex cache when full

Serhiy Storchaka added the comment: Nice catch! Here is a patch that deletes the first key. -- stage: commit review -> patch review Added file: http://bugs.python.org/file44853/re_cache_del_first.patch ___ Python tracker

[issue28293] Don't completely dump the regex cache when full

Xiang Zhang added the comment: But with the compact dict implementation, popitem is not going to evict an arbitrary entry but always the last one. Will this cause two interchangeably used regexes always need to recompile? -- nosy: +xiang.zhang ___

[issue28293] Don't completely dump the regex cache when full

Serhiy Storchaka added the comment: LGTM. -- assignee: serhiy.storchaka -> rhettinger stage: patch review -> commit review ___ Python tracker ___

[issue28293] Don't completely dump the regex cache when full

New submission from Raymond Hettinger: When the regex cache gets filled, it is cleared in its entirety. Instead, it only needs to evict one arbitrary entry. This will save the us from having to rebuild and recache frequently used regexes. -- assignee: serhiy.storchaka components: