Skip to content

Commit f2544aa

Browse files
tbkkatbkka
authored
Release node factory storage after each demangling operation (swiftlang#35961)
* Release node factory storage after each demangling operation This adds missing clear() operations to a number of places in RemoteMirror in order to reclaim memory after (de)mangling results are no longer needed. Before this, the RemoteMirror library had an unfortunate tendency to use excessive amounts of memory as the demangler kept appending data to its internal slab allocator. Resolves rdar://72958641 * Include payload cases even if we cannot retrieve the typeinfo Otherwise, we end up with inconsistent counts of payload and non-payload cases, which screws up some of the enum management. * Add a very basic check of enum with CF payload.
1 parent 0948105 commit f2544aa

File tree

4 files changed

+94
-11
lines changed

4 files changed

+94
-11
lines changed

‎include/swift/Reflection/TypeRefBuilder.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,8 @@ class TypeRefBuilder {
287287

288288
Demangle::NodeFactory &getNodeFactory() { return Dem; }
289289

290+
void clearNodeFactory() { Dem.clear(); }
291+
290292
BuiltType decodeMangledType(Node *node);
291293

292294
///

‎stdlib/public/Reflection/TypeLowering.cpp

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ class NoPayloadEnumTypeInfo: public EnumTypeInfo {
393393
/*BitwiseTakable*/ true,
394394
EnumKind::NoPayloadEnum, Cases) {
395395
assert(Cases.size() >= 2);
396-
// assert(getNumPayloadCases() == 0);
396+
assert(getNumPayloadCases() == 0);
397397
}
398398

399399
bool readExtraInhabitantIndex(remote::MemoryReader &reader,
@@ -437,7 +437,7 @@ class SinglePayloadEnumTypeInfo: public EnumTypeInfo {
437437
: EnumTypeInfo(Size, Alignment, Stride, NumExtraInhabitants,
438438
BitwiseTakable, EnumKind::SinglePayloadEnum, Cases) {
439439
assert(Cases[0].TR != 0);
440-
// assert(getNumPayloadCases() == 1);
440+
assert(getNumPayloadCases() == 1);
441441
}
442442

443443
bool readExtraInhabitantIndex(remote::MemoryReader &reader,
@@ -555,6 +555,7 @@ class SimpleMultiPayloadEnumTypeInfo: public EnumTypeInfo {
555555
assert(Cases[1].TR != 0);
556556
assert(getNumPayloadCases() > 1);
557557
assert(getSize() > getPayloadSize());
558+
assert(getCases().size() > 1);
558559
}
559560

560561
bool readExtraInhabitantIndex(remote::MemoryReader &reader,
@@ -1766,14 +1767,14 @@ class EnumTypeInfoBuilder {
17661767
if (TI == nullptr) {
17671768
DEBUG_LOG(fprintf(stderr, "No TypeInfo for case type: "); TR->dump());
17681769
Invalid = true;
1769-
return;
1770+
static TypeInfo emptyTI;
1771+
Cases.push_back({Name, /*offset=*/0, /*value=*/-1, TR, emptyTI});
1772+
} else {
1773+
Size = std::max(Size, TI->getSize());
1774+
Alignment = std::max(Alignment, TI->getAlignment());
1775+
BitwiseTakable &= TI->isBitwiseTakable();
1776+
Cases.push_back({Name, /*offset=*/0, /*value=*/-1, TR, *TI});
17701777
}
1771-
1772-
Size = std::max(Size, TI->getSize());
1773-
Alignment = std::max(Alignment, TI->getAlignment());
1774-
BitwiseTakable &= TI->isBitwiseTakable();
1775-
1776-
Cases.push_back({Name, /*offset=*/0, /*value=*/-1, TR, *TI});
17771778
}
17781779

17791780
public:
@@ -1800,6 +1801,7 @@ class EnumTypeInfoBuilder {
18001801
} else {
18011802
PayloadCases.push_back(Case);
18021803
auto *CaseTR = getCaseTypeRef(Case);
1804+
assert(CaseTR != nullptr);
18031805
auto *CaseTI = TC.getTypeInfo(CaseTR, ExternalTypeInfo);
18041806
addCase(Case.Name, CaseTR, CaseTI);
18051807
}

‎stdlib/public/Reflection/TypeRefBuilder.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,9 @@ std::string
8888
TypeRefBuilder::normalizeReflectionName(RemoteRef<char> reflectionName) {
8989
// Remangle the reflection name to resolve symbolic references.
9090
if (auto node = demangleTypeRef(reflectionName)) {
91-
return mangleNode(node);
91+
auto result = mangleNode(node);
92+
clearNodeFactory();
93+
return result;
9294
}
9395

9496
// Fall back to the raw string.
@@ -140,6 +142,7 @@ lookupTypeWitness(const std::string &MangledTypeName,
140142
AssocTy->SubstitutedTypeName);
141143
auto Demangled = demangleTypeRef(SubstitutedTypeName);
142144
auto *TypeWitness = decodeMangledType(Demangled);
145+
clearNodeFactory();
143146

144147
AssociatedTypeCache.insert(std::make_pair(key, TypeWitness));
145148
return TypeWitness;
@@ -159,6 +162,7 @@ const TypeRef *TypeRefBuilder::lookupSuperclass(const TypeRef *TR) {
159162

160163
auto Demangled = demangleTypeRef(readTypeRef(FD, FD->Superclass));
161164
auto Unsubstituted = decodeMangledType(Demangled);
165+
clearNodeFactory();
162166
if (!Unsubstituted)
163167
return nullptr;
164168

@@ -192,7 +196,6 @@ TypeRefBuilder::getFieldTypeInfo(const TypeRef *TR) {
192196
auto CandidateMangledName = readTypeRef(FD, FD->MangledTypeName);
193197
auto NormalizedName = normalizeReflectionName(CandidateMangledName);
194198
FieldTypeInfoCache[NormalizedName] = FD;
195-
Dem.clear();
196199
}
197200
}
198201

@@ -230,6 +233,7 @@ bool TypeRefBuilder::getFieldTypeRefs(
230233

231234
auto Demangled = demangleTypeRef(readTypeRef(Field,Field->MangledTypeName));
232235
auto Unsubstituted = decodeMangledType(Demangled);
236+
clearNodeFactory();
233237
if (!Unsubstituted)
234238
return false;
235239

@@ -308,6 +312,7 @@ TypeRefBuilder::getClosureContextInfo(RemoteRef<CaptureDescriptor> CD) {
308312
auto MangledName = readTypeRef(CR, CR->MangledTypeName);
309313
auto DemangleTree = demangleTypeRef(MangledName);
310314
TR = decodeMangledType(DemangleTree);
315+
clearNodeFactory();
311316
}
312317
Info.CaptureTypes.push_back(TR);
313318
}
@@ -320,6 +325,7 @@ TypeRefBuilder::getClosureContextInfo(RemoteRef<CaptureDescriptor> CD) {
320325
auto MangledName = readTypeRef(MSR, MSR->MangledTypeName);
321326
auto DemangleTree = demangleTypeRef(MangledName);
322327
TR = decodeMangledType(DemangleTree);
328+
clearNodeFactory();
323329
}
324330

325331
const MetadataSource *MS = nullptr;
@@ -348,6 +354,7 @@ TypeRefBuilder::dumpTypeRef(RemoteRef<char> MangledName,
348354
auto TypeName = nodeToString(DemangleTree);
349355
fprintf(file, "%s\n", TypeName.c_str());
350356
auto Result = swift::Demangle::decodeMangledType(*this, DemangleTree);
357+
clearNodeFactory();
351358
if (Result.isError()) {
352359
auto *Error = Result.getError();
353360
char *ErrorStr = Error->copyErrorString();
@@ -368,6 +375,7 @@ void TypeRefBuilder::dumpFieldSection(FILE *file) {
368375
auto TypeDemangling =
369376
demangleTypeRef(readTypeRef(descriptor, descriptor->MangledTypeName));
370377
auto TypeName = nodeToString(TypeDemangling);
378+
clearNodeFactory();
371379
fprintf(file, "%s\n", TypeName.c_str());
372380
for (size_t i = 0; i < TypeName.size(); ++i)
373381
fprintf(file, "-");
@@ -396,6 +404,7 @@ void TypeRefBuilder::dumpAssociatedTypeSection(FILE *file) {
396404
auto protocolNode = demangleTypeRef(
397405
readTypeRef(descriptor, descriptor->ProtocolTypeName));
398406
auto protocolName = nodeToString(protocolNode);
407+
clearNodeFactory();
399408

400409
fprintf(file, "- %s : %s", conformingTypeName.c_str(), protocolName.c_str());
401410
fprintf(file, "\n");
@@ -420,6 +429,7 @@ void TypeRefBuilder::dumpBuiltinTypeSection(FILE *file) {
420429
auto typeNode = demangleTypeRef(readTypeRef(descriptor,
421430
descriptor->TypeName));
422431
auto typeName = nodeToString(typeNode);
432+
clearNodeFactory();
423433

424434
fprintf(file, "\n- %s:\n", typeName.c_str());
425435
fprintf(file, "Size: %u\n", descriptor->Size);

‎validation-test/Reflection/reflect_Enum_SingleCaseCFPayload.swift

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift -g -lswiftSwiftReflectionTest %s -o %t/reflect_Enum_SingleCaseCFPayload
3+
// RUN: %target-codesign %t/reflect_Enum_SingleCaseCFPayload
4+
5+
// RUN: %target-run %target-swift-reflection-test %t/reflect_Enum_SingleCaseCFPayload | %FileCheck %s --check-prefix=CHECK-%target-ptrsize
6+
7+
// REQUIRES: objc_interop
8+
// REQUIRES: reflection_test_support
9+
// REQUIRES: executable_test
10+
// UNSUPPORTED: use_os_stdlib
11+
12+
import SwiftReflectionTest
13+
import Foundation
14+
15+
enum SingleCaseCFPayloadEnum {
16+
case only(CFString)
17+
}
18+
19+
let cfs1 = "1" as CFString
20+
let cfs2 = "2" as CFString
21+
22+
class ClassWithSingleCaseCFPayloadEnum {
23+
var e1: SingleCaseCFPayloadEnum?
24+
var e2: SingleCaseCFPayloadEnum = .only(cfs1)
25+
var e3: SingleCaseCFPayloadEnum? = .some(.only(cfs2))
26+
var e4: SingleCaseCFPayloadEnum??
27+
}
28+
29+
reflect(object: ClassWithSingleCaseCFPayloadEnum())
30+
31+
// CHECK-64: Reflecting an object.
32+
// CHECK-64: Instance pointer in child address space: 0x{{[0-9a-fA-F]+}}
33+
// CHECK-64: Type reference:
34+
// CHECK-64: (class reflect_Enum_SingleCaseCFPayload.ClassWithSingleCaseCFPayloadEnum)
35+
// CHECK-64: Type info:
36+
// CHECK-64: <null type info>
37+
38+
// CHECK-32: Reflecting an object.
39+
// CHECK-32: Instance pointer in child address space: 0x{{[0-9a-fA-F]+}}
40+
// CHECK-32: Type reference:
41+
// CHECK-32: (class reflect_Enum_SingleCaseCFPayload.ClassWithSingleCaseCFPayloadEnum)
42+
// CHECK-32: Type info:
43+
// CHECK-32: <null type info>
44+
45+
reflect(enum: SingleCaseCFPayloadEnum.only(cfs1))
46+
47+
// CHECK-64: Reflecting an enum.
48+
// CHECK-64: Instance pointer in child address space: 0x{{[0-9a-fA-F]+}}
49+
// CHECK-64: Type reference:
50+
// CHECK-64: (enum reflect_Enum_SingleCaseCFPayload.SingleCaseCFPayloadEnum)
51+
// CHECK-64: Type info:
52+
// CHECK-64: <null type info>
53+
// CHECK-64: Enum value:
54+
// CHECK-64: <null type info>
55+
56+
// CHECK-32: Reflecting an enum.
57+
// CHECK-32: Instance pointer in child address space: 0x{{[0-9a-fA-F]+}}
58+
// CHECK-32: Type reference:
59+
// CHECK-32: (enum reflect_Enum_SingleCaseCFPayload.SingleCaseCFPayloadEnum)
60+
// CHECK-32: Type info:
61+
// CHECK-32: <null type info>
62+
// CHECK-32: Enum value:
63+
// CHECK-32: <null type info>
64+
65+
doneReflecting()
66+
67+
// CHECK-64: Done.
68+
69+
// CHECK-32: Done.

0 commit comments

Comments
 (0)