[issue35780] Recheck logic in the C version of the lru_cache()

Change by Raymond Hettinger : -- pull_requests: +11630, 11631, 11632 ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue35780] Recheck logic in the C version of the lru_cache()

Change by Raymond Hettinger : -- pull_requests: +11630, 11631 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35780] Recheck logic in the C version of the lru_cache()

Change by Raymond Hettinger : -- pull_requests: +11630 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35780] Recheck logic in the C version of the lru_cache()

Change by miss-islington : -- pull_requests: +11579, 11580 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35780] Recheck logic in the C version of the lru_cache()

Change by miss-islington : -- pull_requests: +11579, 11580, 11581 ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue35780] Recheck logic in the C version of the lru_cache()

Change by miss-islington : -- pull_requests: +11579 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35780] Recheck logic in the C version of the lru_cache()

Change by Raymond Hettinger : -- pull_requests: +11576, 11577 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35780] Recheck logic in the C version of the lru_cache()

Change by Raymond Hettinger : -- pull_requests: +11576 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35780] Recheck logic in the C version of the lru_cache()

Change by Raymond Hettinger : -- pull_requests: +11576, 11577, 11578 ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue35780] Recheck logic in the C version of the lru_cache()

Change by Raymond Hettinger : -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker ___

[issue35780] Recheck logic in the C version of the lru_cache()

Raymond Hettinger added the comment: New changeset b2b023c657ba8c3f4a24d0c847d10fe8e2a73d44 by Raymond Hettinger (Miss Islington (bot)) in branch '3.7': bpo-35780: Fix errors in lru_cache() C code (GH-11623) (GH-11682)

[issue35780] Recheck logic in the C version of the lru_cache()

Change by miss-islington : -- pull_requests: +11515, 11516 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35780] Recheck logic in the C version of the lru_cache()

Change by miss-islington : -- pull_requests: +11515 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35780] Recheck logic in the C version of the lru_cache()

Raymond Hettinger added the comment: New changeset d8080c01195cc9a19af752bfa04d98824dd9fb15 by Raymond Hettinger in branch 'master': bpo-35780: Fix errors in lru_cache() C code (GH-11623) https://github.com/python/cpython/commit/d8080c01195cc9a19af752bfa04d98824dd9fb15 --

[issue35780] Recheck logic in the C version of the lru_cache()

Raymond Hettinger added the comment: > I am not sure about an addition _PyDict_GetItem_KnownHash(). It is a necessary check. The user call is allowed to update the cache so we no longer know without checking whether the new key/result pair has already been added. That is in fact the main

[issue35780] Recheck logic in the C version of the lru_cache()

Serhiy Storchaka added the comment: For the test you can use "nonlocal". -- ___ Python tracker ___ ___ Python-bugs-list mailing

[issue35780] Recheck logic in the C version of the lru_cache()

Serhiy Storchaka added the comment: It was the explanation of possible history, not the justification of bugs. Of course bugs should be fixed. Thank you for rechecking this code and for your fix. As for the optimization in lru_cache_make_key(), consider the following example: @lru_cache()

[issue35780] Recheck logic in the C version of the lru_cache()

Change by Ned Deily : -- versions: -Python 3.6 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35780] Recheck logic in the C version of the lru_cache()

Change by Raymond Hettinger : -- keywords: +patch, patch, patch pull_requests: +11378, 11379, 11380 stage: -> patch review ___ Python tracker ___

[issue35780] Recheck logic in the C version of the lru_cache()

Change by Raymond Hettinger : -- keywords: +patch pull_requests: +11378 stage: -> patch review ___ Python tracker ___ ___

[issue35780] Recheck logic in the C version of the lru_cache()

Change by Raymond Hettinger : -- keywords: +patch, patch pull_requests: +11378, 11379 stage: -> patch review ___ Python tracker ___

[issue35780] Recheck logic in the C version of the lru_cache()

Raymond Hettinger added the comment: > It was discussed before, and there is a closed issue. That is a non-answer. The above patch is correct and achieves an essential goal of the lru_cache to save space when possible (avoid an unnecessary extra tuple per entry). Also, please apply the

[issue35780] Recheck logic in the C version of the lru_cache()

Serhiy Storchaka added the comment: Operations with the linked list are atomic (guarded with the GIL), while operations with the cache dict are not. That is why links are removed first from the linked list and added back in case of error. -- ___

[issue35780] Recheck logic in the C version of the lru_cache()

Serhiy Storchaka added the comment: > If so, then why is the link being moved to the front of the lru_cache -- it > should have remained at the oldest position. It may be unintentionally. In any case, this is a case that should be very rare. > The solution to this is only extract the link

[issue35780] Recheck logic in the C version of the lru_cache()

Serhiy Storchaka added the comment: > After the check for popresult==Py_None, there is the comment that was mostly > copied from the Python version but doesn't match the actual code: Seems the comment was placed at wrong place. And it should be updated since locks are not used, but the GIL.

[issue35780] Recheck logic in the C version of the lru_cache()

Serhiy Storchaka added the comment: > would it be reasonable to emulate the pure python code and return a scalar > instead of a tuple when the tuple length is one and there are no keyword > arguments or typing requirements? It was discussed before, and there is a closed issue. I am not sure

[issue35780] Recheck logic in the C version of the lru_cache()

Raymond Hettinger added the comment: - Demonstration of one of the bugs - # The currsize is initially equal to maxsize of 10 # Then we cause an orphan link in a full cache # The currsize drops to 9 and never recovers the full size of 10 from functools import lru_cache once

[issue35780] Recheck logic in the C version of the lru_cache()

Raymond Hettinger added the comment: Suggested code for the open question listed above: --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -733,6 +733,15 @@ lru_cache_make_key(PyObject *args, PyObject *kwds, int typed) /* short path, key will match args anyway, which

[issue35780] Recheck logic in the C version of the lru_cache()

New submission from Raymond Hettinger : After the check for popresult==Py_None, there is the comment that was mostly copied from the Python version but doesn't match the actual code: /* Getting here means that this same key was added to the cache while the lock was released. Since the