Skip to content

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Sep 26, 2017

@serhiy-storchaka serhiy-storchaka merged commit 114454e into python:master Sep 26, 2017
@serhiy-storchaka serhiy-storchaka deleted the re_cache_ordered_dict_popitem branch September 26, 2017 16:47
@@ -281,7 +288,10 @@ def _compile(pattern, flags):
p = sre_compile.compile(pattern, flags)
if not (flags & DEBUG):
if len(_cache) >= _MAXCACHE:
_cache.clear()
try:
_cache.popitem(False)
Copy link
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.)

Copy link
Member Author

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.

Copy link
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.

Copy link
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants