| msg135540 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2011-05-08 20:00 |
| The code for check_GetFinalPathNameByHandle() goes like this: static int has_GetFinalPathNameByHandle = 0; [...] static int check_GetFinalPathNameByHandle() { HINSTANCE hKernel32; /* only recheck */ if (!has_GetFinalPathNameByHandle) { [...] has_GetFinalPathNameByHandle = Py_GetFinalPathNameByHandleA && Py_GetFinalPathNameByHandleW; } return has_GetFinalPathNameByHandle; } It means that if the computed value is 0 (system doesn't have GetFinalPathNameByHandle*), the value will be re-computed each time the function is called. has_GetFinalPathNameByHandle should probably be tri-state instead (0, 1, unknown). |
|
|
| msg136123 - (view) |
Author: Sijin Joseph (sijinjoseph) |
Date: 2011-05-16 20:00 |
| Is this related to some other issue? The fix seems trivial, however I am curious as to how you stumbled upon this? Is there more to this issue than just performance? |
|
|
| msg136124 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2011-05-16 20:02 |
| Yes, it should be trivial to fix. I've stumbled on this simply because I needed to write similar code for another patch, so studied how it was done in posixmodule.c. |
|
|
| msg136789 - (view) |
Author: Catalin Iacob (catalin.iacob) * |
Date: 2011-05-24 20:19 |
| I looked at providing a patch for this issue as an introductory step (this would be my first patch). But when following the code I discovered that the case that this issue is trying to optimize is never executed in current CPython. In that case, there isn't much value in optimizing it. More precisely, check_GetFinalPathNameByHandle is called by posix__getfinalpathname which is nt._getfinalpathname in Python code. If the check fails, posix__getfinalpathname throws NotImplmenentedError. But nt._getfinalpathname is only used by ntpath.py which checks the Windows version and only calls nt._getfinalpathname for Vista and higher where the check won't fail. To me it would make more sense that the nt module has a _getfinalpathname attribute only if it supports the feature instead of always having one that throws NotImplementedError. In that case, ntpath.py would not check the Windows version but the presence of _getfinalpathname in the nt module. Does this seem like a better approach to you, at least theoretically? And if so, is it worth implementing it? Thanks for any advice. |
|
|
| msg166540 - (view) |
Author: Atsuo Ishimoto (ishimoto) * |
Date: 2012-07-27 04:00 |
| Patch to cache result of check_GetFinalPathNameByHandle(). |
|
|
| msg166545 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-07-27 06:45 |
| Thank you, patch looks fine to me. It will have to wait for after the 3.3 release, though, since we are now in feature freeze. |
|
|
| msg173451 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-10-21 14:35 |
| New changeset 8afa3ce5ff3e by Antoine Pitrou in branch 'default': Issue #12034: Fix bogus caching of result in check_GetFinalPathNameByHandle. http://hg.python.org/cpython/rev/8afa3ce5ff3e |
|
|
| msg173452 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-10-21 14:42 |
| Patch committed, thank you! |
|
|