Skip to content

bpo-28293: The regex cache no longer completely dump when full. - #3768

Merged
serhiy-storchaka merged 2 commits into
python:masterfrom
serhiy-storchaka:re_cache_ordered_dict_popitem
Sep 26, 2017
Merged

bpo-28293: The regex cache no longer completely dump when full.#3768
serhiy-storchaka merged 2 commits into
python:masterfrom
serhiy-storchaka:re_cache_ordered_dict_popitem

Conversation

@serhiy-storchaka

@serhiy-storchakaserhiy-storchaka commented Sep 26, 2017•
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

@serhiy-storchakaserhiy-storchaka added the type-feature A feature request or enhancement label Sep 26, 2017
@serhiy-storchaka
serhiy-storchaka merged commit 114454e into python:masterSep 26, 2017
@serhiy-storchaka
serhiy-storchaka deleted the re_cache_ordered_dict_popitem branch September 26, 2017 16:47
warsaw
warsaw reviewed Sep 27, 2017
View reviewed changes
Comment threadLib/re.py
if len(_cache) >= _MAXCACHE:
_cache.clear()
try:
_cache.popitem(False)

@warsawwarsawSep 27, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry that I'm late to the game in reviewing this, and thanks for fixing this. One comment, should this be

_cache.popitem(last=False)

for readability? (I think if history had been different, we would have made that a keyword-only argument.)

@serhiy-storchakaserhiy-storchakaSep 27, 2017

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe. This wouldn't harm performance, because sre_compile.compile() few lines above is much more expensive. Open a new PR for changing this if you will.

@warsawwarsawSep 27, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm reworking my deferred compilation branch to add a new function, so I might just fold this simple change into that branch.

@warsawwarsawSep 27, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind; I'm abandoning the deferred compilation branch, so #3791

@warsawwarsaw mentioned this pull request Sep 27, 2017
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@warsawwarsawwarsaw left review comments

Assignees

No one assigned

Labels

type-featureA feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@serhiy-storchaka@warsaw@Mariatta@the-knights-who-say-ni@bedevere-bot