Skip to content

Commit 1fff98c

Browse files
committed
Rewrite the MRTG framework, and fix some associated serialization issues.
Prior to this change, the MRTG (MutuallyRecursiveTypeGroup) framework was not threadsafe. What was happening was that if two threads both attempted to hash a big group of types, such as a large class with many functions in it, there was a race condition where one thread would check in the entire group while the other was still traversing it. The other group would then construct a second group consisting of the types it had visited, but none of the ones that it had not seen but that had just been installed, and this could lead to two copies of the type existing with different hashes (since this can happen during deserialization of type groups). This change fixes that by rewriting the entire hashing and MRTG framework to have a disciplined model for how threads interact while they are hashing the same objects and types, and for how our framework walks the python and Type* graph. Prior to this change, we had two different ways of walking the graph for hashing purposes - one for discovery (CompilerVisibleObjectVisitor) and one for hashing (Type::computeIdentityHash). Now, we have one way of walking objects, which is owned by CompilerVisibleObjectVisitor, who invokes a Type's visitCompilerVisibleInternals method to walk the internals. This allows for a single unified framework where we are guaranteed that the same object relations that are used to determine the graph structure of the MRTG are also used in hashing. We also ensured that we don't have any Instance objects floating around in this framework - everything is either a PyObject* or a Type*, encapsulated by instances of TypeOrPyobj. Finally, we correct a number of serious errors in the serialization/deserialization layer that were sliding undetected under the prior framework. In particular, we're careful to preserve the "Forward" structure (since Forward instances can sometimes make their way into modules), and to be sure that we are consistent about how we call 'representationFor' and 'nameForObject' when serializing, for both Type* and PyObject*. We also introduce SerializationContext.factoryFor to handle TypeFunction classes, since these, like nameForObject, can be uniquely identified by a much simpler-to-deserialize set of objects.
1 parent e9b8a94 commit 1fff98c

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+1826
-1622
lines changed

typed_python/AlternativeMatcherType.hpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,10 @@ class AlternativeMatcher : public Type {
3232
endOfConstructorInitialization(); // finish initializing the type object.
3333
}
3434

35-
ShaHash _computeIdentityHash(MutuallyRecursiveTypeGroup* groupHead = nullptr) {
36-
return ShaHash(1, m_typeCategory) + m_alternative->identityHash(groupHead);
35+
template<class visitor_type>
36+
void _visitCompilerVisibleInternals(const visitor_type& v) {
37+
v.visitHash(ShaHash(1, m_typeCategory));
38+
v.visitTopo(m_alternative);
3739
}
3840

3941
template<class visitor_type>

typed_python/AlternativeType.cpp

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,6 @@ int64_t Alternative::refcount(instance_ptr i) const {
5050
return ((layout**)i)[0]->refcount;
5151
}
5252

53-
ShaHash Alternative::_computeIdentityHash(MutuallyRecursiveTypeGroup* groupHead) {
54-
ShaHash newHash(1, m_typeCategory);
55-
56-
newHash = newHash + ShaHash(m_name) + ShaHash(0);
57-
for (auto& subtype_pair: m_subtypes) {
58-
newHash = newHash + ShaHash(subtype_pair.first) + subtype_pair.second->identityHash(groupHead);
59-
}
60-
newHash = newHash + ShaHash(1);
61-
for (auto nameAndMethod: m_methods) {
62-
newHash = newHash + ShaHash(nameAndMethod.first) + nameAndMethod.second->identityHash(groupHead);
63-
}
64-
return newHash;
65-
}
66-
6753
bool Alternative::_updateAfterForwardTypesChanged() {
6854
m_arg_positions.clear();
6955
m_default_construction_type = nullptr;

typed_python/AlternativeType.hpp

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,23 @@ class Alternative : public Type {
157157

158158
bool _updateAfterForwardTypesChanged();
159159

160-
ShaHash _computeIdentityHash(MutuallyRecursiveTypeGroup* groupHead = nullptr);
160+
template<class visitor_type>
161+
void _visitCompilerVisibleInternals(const visitor_type& v) {
162+
v.visitHash(ShaHash(1, m_typeCategory));
163+
v.visitName(m_name);
164+
v.visitHash(ShaHash(m_subtypes.size()));
165+
166+
for (auto& subtype_pair: m_subtypes) {
167+
v.visitNamedTopo(subtype_pair.first, subtype_pair.second);
168+
}
169+
170+
v.visitHash(ShaHash(m_methods.size()));
171+
172+
for (auto nameAndMethod: m_methods) {
173+
v.visitNamedTopo(nameAndMethod.first, nameAndMethod.second);
174+
}
175+
}
176+
161177

162178
bool cmp(instance_ptr left, instance_ptr right, int pyComparisonOp, bool suppressExceptions);
163179
static bool cmpStatic(Alternative* altT, instance_ptr left, instance_ptr right, int64_t pyComparisonOp);

typed_python/BoundMethodType.hpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,11 @@ class BoundMethod : public Type {
3232
endOfConstructorInitialization(); // finish initializing the type object.
3333
}
3434

35-
ShaHash _computeIdentityHash(MutuallyRecursiveTypeGroup* groupHead = nullptr) {
36-
return (
37-
ShaHash(1, m_typeCategory) +
38-
m_first_arg->identityHash(groupHead) +
39-
ShaHash(m_funcName)
40-
);
35+
template<class visitor_type>
36+
void _visitCompilerVisibleInternals(const visitor_type& v) {
37+
v.visitHash(ShaHash(1, m_typeCategory));
38+
v.visitTopo(m_first_arg);
39+
v.visitName(m_funcName);
4140
}
4241

4342
template<class visitor_type>

typed_python/BytesType.hpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,11 @@ class BytesType : public Type {
143143
template<class visitor_type>
144144
void _visitContainedTypes(const visitor_type& v) {}
145145

146+
template<class visitor_type>
147+
void _visitCompilerVisibleInternals(const visitor_type& v) {
148+
v.visitHash(ShaHash(1, m_typeCategory));
149+
}
150+
146151
typed_python_hash_type hash(instance_ptr left);
147152

148153
bool cmp(instance_ptr left, instance_ptr right, int pyComparisonOp, bool suppressExceptions);

typed_python/ClassType.hpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,10 @@ class Class : public Type {
187187

188188
bool checkInitializationFlag(instance_ptr self, int64_t ix) const;
189189

190-
ShaHash _computeIdentityHash(MutuallyRecursiveTypeGroup* groupHead = nullptr) {
191-
return ShaHash(1, m_typeCategory) + m_heldClass->identityHash(groupHead);
190+
template<class visitor_type>
191+
void _visitCompilerVisibleInternals(const visitor_type& v) {
192+
v.visitHash(ShaHash(1, m_typeCategory));
193+
v.visitTopo(m_heldClass);
192194
}
193195

194196
bool cmp(instance_ptr left, instance_ptr right, int pyComparisonOp, bool suppressExceptions);

0 commit comments

Comments
 (0)