You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
importlib.reload not updated when only change a little bit code layout #120932
At first glance, this looks like a problem related to cache invalidation inside importlib, which would explain why it doesn't occur when comments are added. I'm guessing it does this by the size of the file, since this problem only occurs when the original file size is equivalent to the new one (which is rare, but still a bug).
I played around with this a little bit, and it seems likely that the problem is with SourceLoader.get_code -- only the file is updated, not the compiled pyc file. get_code then reads that old compiled file, and ignores the updated py file.
The problem is here. The repro updates the file too quickly for the number to be different, and when the size is the same, it doesn't get caught there either.
I'm not all too sure this is worth fixing, considering it will probably have negative performance benefits, and only happens in very extreme cases. It might be worth documenting, though. What do you think, @serhiy-storchaka (pinging you considering you helped on the previous zipimport issue)?
it can be worked around by considering the .py file over bytecode when the timestamp second value is the same, at the cost of sometimes not using bytecode when it could have been
This is possible, but this may be not so simple. Currently the code compares the timestamp of the .py file (truncated to seconds) with the timestamp saved in the .pyc file. They are supposed to be equal. You need a third timestamp to handle the case when the .py file was changed within a second.
Use the timestamp of the .pyc file. It needs to be strongly larger than the timestamp of the .py file (truncated to seconds) to ensure that the .pyc file is not outdated. But additional stat() call adds an overhead. We are trying to minimize the number of system calls for import.
Use the current time. The problem is that on the FAT filesystem the file timestamp has granularity of 2 seconds. So the timestamp of just written file will often be different from the current time truncated to seconds.. And you cannot easily say what is the granularity of the file timestamp. There is also lesser problem of double rounding on filesystems with more precise file timestamps.
Hybrid approach. If the cost of stat() is significant, we can use the current time, and only if the difference between timestamps is small, use the timestamp of the .pyc file. It will be not easy to test.
Activity
ZeroIntensity commentedon Jul 11, 2024
This is confirmed on Python 3.12 and the CPython main branch. Here's a smaller reproducer:
At first glance, this looks like a problem related to cache invalidation inside
importlib, which would explain why it doesn't occur when comments are added. I'm guessing it does this by the size of the file, since this problem only occurs when the original file size is equivalent to the new one (which is rare, but still a bug).I played around with this a little bit, and it seems likely that the problem is with
SourceLoader.get_code-- only the file is updated, not the compiledpycfile.get_codethen reads that old compiled file, and ignores the updatedpyfile.I'll submit a patch for this ASAP.
ZeroIntensity commentedon Jul 11, 2024
The problem is here. The repro updates the file too quickly for the number to be different, and when the size is the same, it doesn't get caught there either.
I'm not all too sure this is worth fixing, considering it will probably have negative performance benefits, and only happens in very extreme cases. It might be worth documenting, though. What do you think, @serhiy-storchaka (pinging you considering you helped on the previous
zipimportissue)?serhiy-storchaka commentedon Jul 11, 2024
This is not related to
zipimport, but it looks like the same issue as #121376 (there may be other open issues for the same problem).pythongh-120932: Chack the pyc file mtime at import
pythongh-120932: Check the pyc file mtime at import
pythongh-120932: Check the pyc file mtime at import
serhiy-storchaka commentedon Jul 11, 2024
Repeating myself from #121376 (comment):
This is possible, but this may be not so simple. Currently the code compares the timestamp of the
.pyfile (truncated to seconds) with the timestamp saved in the.pycfile. They are supposed to be equal. You need a third timestamp to handle the case when the.pyfile was changed within a second..pycfile. It needs to be strongly larger than the timestamp of the.pyfile (truncated to seconds) to ensure that the.pycfile is not outdated. But additionalstat()call adds an overhead. We are trying to minimize the number of system calls for import.stat()is significant, we can use the current time, and only if the difference between timestamps is small, use the timestamp of the.pycfile. It will be not easy to test.serhiy-storchaka commentedon Jul 11, 2024
#121620 implements this idea.
ZeroIntensity commentedon Jul 11, 2024
Right, I was just assuming you had knowledge of the import system :)