fix[tool]: update VarAccess pickle implementation#4270
fix[tool]: update VarAccess pickle implementation#4270charles-cooper merged 9 commits intovyperlang:masterfrom
Conversation
|
can we add an explainer comment on why's this necessary? |
will get to it |
| # see https://github.com/python/cpython/issues/124937#issuecomment-2392227290 | ||
| def __reduce__(self): | ||
| dict_obj = {f.name: getattr(self, f.name) for f in fields(self)} | ||
| return self.__class__._produce, (dict_obj,) |
There was a problem hiding this comment.
You could also use the following trick if all fields can be passed as keyword arguments to constructor in subclasses:
| return self.__class__._produce, (dict_obj,) | |
| return partial(self.__class__, **dict_obj), () |
There was a problem hiding this comment.
interesting, is there a performance difference?
There was a problem hiding this comment.
It may be slower. The advantage is that you do not need to add private method _produce().
|
copying in some offline discussion with @cyberthirst about why the hash is safe even though the data structure is cyclic:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4270 +/- ##
==========================================
- Coverage 91.30% 90.07% -1.23%
==========================================
Files 110 110
Lines 15764 15770 +6
Branches 3461 3463 +2
==========================================
- Hits 14393 14205 -188
- Misses 935 1095 +160
- Partials 436 470 +34 ☔ View full report in Codecov by Sentry. |
What I did
fix a bug where unpickling
annotated_vyper_modulewould lead to a crash:a repro can be found at charles-cooper@2a5facf.
this is a blocker for tooling, for instance titanoboa relies on pickle/unpickle to cache CompilerData objects:
https://github.com/vyperlang/titanoboa/blob/86df8936654db20686410488738d7abaf165a4c9/boa/util/disk_cache.py#L65-L66
the underlying issue is that
pickle.loads()callsobj.__hash__()for objects that are keys in a hashed data structure - namely, dicts, sets and frozensets. this causes a crash when there is a cycle in the object graph, because the object is not fully instantiated at the time that__hash__()is called. this is a cpython bug, reported at python/cpython#124937.@serhiy-storchaka suggested the approach taken in this PR, which breaks the loop before pickling: python/cpython#124937 (comment).
How I did it
How to verify it
Commit message
Description for the changelog
Cute Animal Picture