[Serialization] Do less work checking if a value can be deserialized. (swiftlang#9666)

jrose-apple · web-flow · commit 4a6fe941c7ac · 2017-05-17T09:02:45.000-07:00
Previously we recorded the canonical type of the declaration and made
sure we could deserialize that, but that's a lot of extra work
building up intermediate types that we mostly don't need. Instead,
record smaller types that represent the possible points of failure---
right now, just the nominal types that are referenced by the value
(function, variable/constant, subscript, or initializer). I chose to
use types instead of declarations here because types can potentially
encode more complicated constraints later (such as generic types
checking that their arguments still conform).

This gains us back 20% of type-checking time on a compile-time
microbenchmark: `let _ = [1, 2]`. I expect the effect is less dramatic
the more expressions you have, since we only need to deserialize
things once.
diff --git a/include/swift/Serialization/ModuleFormat.h b/include/swift/Serialization/ModuleFormat.h
@@ -855,12 +855,13 @@ namespace decls_block {
     CtorInitializerKindField,  // initializer kind
     GenericEnvironmentIDField, // generic environment
     TypeIDField, // interface type
-    TypeIDField, // canonical interface type
     DeclIDField, // overridden decl
     AccessibilityKindField, // accessibility
     BCFixed<1>,   // requires a new vtable slot
     BCFixed<1>,   // 'required' but overridden is not (used for recovery)
-    BCArray<IdentifierIDField> // argument names
+    BCVBR<5>,     // number of parameter name components
+    BCArray<IdentifierIDField> // name components,
+                               // followed by TypeID dependencies
     // Trailed by its generic parameters, if any, followed by the parameter
     // patterns.
   >;
@@ -876,7 +877,6 @@ namespace decls_block {
     BCFixed<1>,   // HasNonPatternBindingInit?
     StorageKindField,   // StorageKind
     TypeIDField,  // interface type
-    TypeIDField,  // canonical interface type
     DeclIDField,  // getter
     DeclIDField,  // setter
     DeclIDField,  // materializeForSet
@@ -886,7 +886,8 @@ namespace decls_block {
     DeclIDField,  // didset
     DeclIDField,  // overridden decl
     AccessibilityKindField, // accessibility
-    AccessibilityKindField // setter accessibility, if applicable
+    AccessibilityKindField, // setter accessibility, if applicable
+    BCArray<TypeIDField> // dependencies
   >;
 
   using ParamLayout = BCRecordLayout<
@@ -911,15 +912,16 @@ namespace decls_block {
     BCVBR<5>,     // number of parameter patterns
     GenericEnvironmentIDField, // generic environment
     TypeIDField,  // interface type
-    TypeIDField,  // canonical interface type
     DeclIDField,  // operator decl
     DeclIDField,  // overridden function
     DeclIDField,  // AccessorStorageDecl
-    BCFixed<1>,   // name is compound?
+    BCVBR<5>,     // 0 for a simple name, otherwise the number of parameter name
+                  // components plus one
     AddressorKindField, // addressor kind
     AccessibilityKindField, // accessibility
     BCFixed<1>,   // requires a new vtable slot
-    BCArray<IdentifierIDField> // name components
+    BCArray<IdentifierIDField> // name components,
+                               // followed by TypeID dependencies
     // The record is trailed by:
     // - its _silgen_name, if any
     // - its generic parameters, if any
@@ -984,7 +986,6 @@ namespace decls_block {
     StorageKindField,   // StorageKind
     GenericEnvironmentIDField, // generic environment
     TypeIDField, // interface type
-    TypeIDField,  // canonical interface type
     DeclIDField, // getter
     DeclIDField, // setter
     DeclIDField, // materializeForSet
@@ -995,7 +996,9 @@ namespace decls_block {
     DeclIDField, // overridden decl
     AccessibilityKindField, // accessibility
     AccessibilityKindField, // setter accessibility, if applicable
-    BCArray<IdentifierIDField> // name components
+    BCVBR<5>,    // number of parameter name components
+    BCArray<IdentifierIDField> // name components,
+                               // followed by TypeID dependencies
     // Trailed by:
     // - generic parameters, if any
     // - the indices pattern
diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp
@@ -2566,25 +2566,27 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
     bool isImplicit, isObjC, hasStubImplementation, throws;
     GenericEnvironmentID genericEnvID;
     uint8_t storedInitKind, rawAccessLevel;
-    TypeID interfaceID, canonicalTypeID;
+    TypeID interfaceID;
     DeclID overriddenID;
     bool needsNewVTableEntry, firstTimeRequired;
-    ArrayRef<uint64_t> argNameIDs;
+    unsigned numArgNames;
+    ArrayRef<uint64_t> argNameAndDependencyIDs;
 
     decls_block::ConstructorLayout::readRecord(scratch, contextID,
                                                rawFailability, isImplicit, 
                                                isObjC, hasStubImplementation,
                                                throws, storedInitKind,
                                                genericEnvID, interfaceID,
-                                               canonicalTypeID, overriddenID,
+                                               overriddenID,
                                                rawAccessLevel,
                                                needsNewVTableEntry,
                                                firstTimeRequired,
-                                               argNameIDs);
+                                               numArgNames,
+                                               argNameAndDependencyIDs);
 
     // Resolve the name ids.
     SmallVector<Identifier, 2> argNames;
-    for (auto argNameID : argNameIDs)
+    for (auto argNameID : argNameAndDependencyIDs.slice(0, numArgNames))
       argNames.push_back(getIdentifier(argNameID));
     DeclName name(ctx, ctx.Id_init, argNames);
 
@@ -2610,10 +2612,12 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
       return llvm::make_error<OverrideError>(name, errorFlags);
     }
 
-    auto canonicalType = getTypeChecked(canonicalTypeID);
-    if (!canonicalType) {
-      return llvm::make_error<TypeError>(
-          name, takeErrorInfo(canonicalType.takeError()), errorFlags);
+    for (auto dependencyID : argNameAndDependencyIDs.slice(numArgNames)) {
+      auto dependency = getTypeChecked(dependencyID);
+      if (!dependency) {
+        return llvm::make_error<TypeError>(
+            name, takeErrorInfo(dependency.takeError()), errorFlags);
+      }
     }
 
     auto parent = getDeclContext(contextID);
@@ -2690,18 +2694,20 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
     DeclContextID contextID;
     bool isImplicit, isObjC, isStatic, isLet, hasNonPatternBindingInit;
     uint8_t storageKind, rawAccessLevel, rawSetterAccessLevel;
-    TypeID interfaceTypeID, canonicalTypeID;
+    TypeID interfaceTypeID;
     DeclID getterID, setterID, materializeForSetID, willSetID, didSetID;
     DeclID addressorID, mutableAddressorID, overriddenID;
+    ArrayRef<uint64_t> dependencyIDs;
 
     decls_block::VarLayout::readRecord(scratch, nameID, contextID,
                                        isImplicit, isObjC, isStatic, isLet,
                                        hasNonPatternBindingInit, storageKind,
-                                       interfaceTypeID, canonicalTypeID,
+                                       interfaceTypeID,
                                        getterID, setterID, materializeForSetID,
                                        addressorID, mutableAddressorID,
                                        willSetID, didSetID, overriddenID,
-                                       rawAccessLevel, rawSetterAccessLevel);
+                                       rawAccessLevel, rawSetterAccessLevel,
+                                       dependencyIDs);
 
     Identifier name = getIdentifier(nameID);
 
@@ -2711,10 +2717,12 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
       return llvm::make_error<OverrideError>(name);
     }
 
-    auto canonicalType = getTypeChecked(canonicalTypeID);
-    if (!canonicalType) {
-      return llvm::make_error<TypeError>(
-          name, takeErrorInfo(canonicalType.takeError()));
+    for (TypeID dependencyID : dependencyIDs) {
+      auto dependency = getTypeChecked(dependencyID);
+      if (!dependency) {
+        return llvm::make_error<TypeError>(
+            name, takeErrorInfo(dependency.takeError()));
+      }
     }
 
     auto DC = ForcedContext ? *ForcedContext : getDeclContext(contextID);
@@ -2802,37 +2810,42 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
     bool isStatic;
     uint8_t rawStaticSpelling, rawAccessLevel, rawAddressorKind;
     bool isObjC, isMutating, hasDynamicSelf, throws;
-    unsigned numParamPatterns;
+    unsigned numParamPatterns, numNameComponentsBiased;
     GenericEnvironmentID genericEnvID;
-    TypeID interfaceTypeID, canonicalTypeID;
+    TypeID interfaceTypeID;
     DeclID associatedDeclID;
     DeclID overriddenID;
     DeclID accessorStorageDeclID;
-    bool hasCompoundName, needsNewVTableEntry;
-    ArrayRef<uint64_t> nameIDs;
+    bool needsNewVTableEntry;
+    ArrayRef<uint64_t> nameAndDependencyIDs;
 
     decls_block::FuncLayout::readRecord(scratch, contextID, isImplicit,
                                         isStatic, rawStaticSpelling, isObjC,
                                         isMutating, hasDynamicSelf, throws,
                                         numParamPatterns, genericEnvID,
-                                        interfaceTypeID, canonicalTypeID,
+                                        interfaceTypeID,
                                         associatedDeclID, overriddenID,
-                                        accessorStorageDeclID, hasCompoundName,
+                                        accessorStorageDeclID,
+                                        numNameComponentsBiased,
                                         rawAddressorKind, rawAccessLevel,
-                                        needsNewVTableEntry, nameIDs);
+                                        needsNewVTableEntry,
+                                        nameAndDependencyIDs);
 
     // Resolve the name ids.
-    SmallVector<Identifier, 2> names;
-    for (auto nameID : nameIDs)
-      names.push_back(getIdentifier(nameID));
-
+    Identifier baseName = getIdentifier(nameAndDependencyIDs.front());
     DeclName name;
-    if (!names.empty()) {
-      if (hasCompoundName)
-        name = DeclName(ctx, names[0],
-                        llvm::makeArrayRef(names.begin() + 1, names.end()));
-      else
-        name = DeclName(names[0]);
+    ArrayRef<uint64_t> dependencyIDs;
+    if (numNameComponentsBiased != 0) {
+      SmallVector<Identifier, 2> names;
+      for (auto nameID : nameAndDependencyIDs.slice(1,
+                                                    numNameComponentsBiased-1)){
+        names.push_back(getIdentifier(nameID));
+      }
+      name = DeclName(ctx, baseName, names);
+      dependencyIDs = nameAndDependencyIDs.slice(numNameComponentsBiased);
+    } else {
+      name = baseName;
+      dependencyIDs = nameAndDependencyIDs.drop_front();
     }
 
     DeclDeserializationError::Flags errorFlags;
@@ -2845,10 +2858,12 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
       return llvm::make_error<OverrideError>(name, errorFlags);
     }
 
-    auto canonicalType = getTypeChecked(canonicalTypeID);
-    if (!canonicalType) {
-      return llvm::make_error<TypeError>(
-          name, takeErrorInfo(canonicalType.takeError()), errorFlags);
+    for (TypeID dependencyID : dependencyIDs) {
+      auto dependency = getTypeChecked(dependencyID);
+      if (!dependency) {
+        return llvm::make_error<TypeError>(
+            name, takeErrorInfo(dependency.takeError()), errorFlags);
+      }
     }
 
     auto DC = getDeclContext(contextID);
@@ -3355,28 +3370,29 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
     DeclContextID contextID;
     bool isImplicit, isObjC;
     GenericEnvironmentID genericEnvID;
-    TypeID interfaceTypeID, canonicalTypeID;
+    TypeID interfaceTypeID;
     DeclID getterID, setterID, materializeForSetID;
     DeclID addressorID, mutableAddressorID, willSetID, didSetID;
     DeclID overriddenID;
     uint8_t rawAccessLevel, rawSetterAccessLevel;
     uint8_t rawStorageKind;
-    ArrayRef<uint64_t> argNameIDs;
+    unsigned numArgNames;
+    ArrayRef<uint64_t> argNameAndDependencyIDs;
 
     decls_block::SubscriptLayout::readRecord(scratch, contextID,
                                              isImplicit, isObjC, rawStorageKind,
                                              genericEnvID,
-                                             interfaceTypeID, canonicalTypeID,
+                                             interfaceTypeID,
                                              getterID, setterID,
                                              materializeForSetID,
                                              addressorID, mutableAddressorID,
                                              willSetID, didSetID,
                                              overriddenID, rawAccessLevel,
-                                             rawSetterAccessLevel,
-                                             argNameIDs);
+                                             rawSetterAccessLevel, numArgNames,
+                                             argNameAndDependencyIDs);
     // Resolve the name ids.
     SmallVector<Identifier, 2> argNames;
-    for (auto argNameID : argNameIDs)
+    for (auto argNameID : argNameAndDependencyIDs.slice(0, numArgNames))
       argNames.push_back(getIdentifier(argNameID));
     DeclName name(ctx, ctx.Id_subscript, argNames);
 
@@ -3386,10 +3402,12 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
       return llvm::make_error<OverrideError>(name);
     }
 
-    auto canonicalType = getTypeChecked(canonicalTypeID);
-    if (!canonicalType) {
-      return llvm::make_error<TypeError>(
-          name, takeErrorInfo(canonicalType.takeError()));
+    for (TypeID dependencyID : argNameAndDependencyIDs.slice(numArgNames)) {
+      auto dependency = getTypeChecked(dependencyID);
+      if (!dependency) {
+        return llvm::make_error<TypeError>(
+            name, takeErrorInfo(dependency.takeError()));
+      }
     }
 
     auto parent = getDeclContext(contextID);
@@ -4379,8 +4397,15 @@ Expected<Type> ModuleFile::getTypeChecked(TypeID TID) {
     decls_block::UnboundGenericTypeLayout::readRecord(scratch,
                                                       genericID, parentID);
 
-    auto genericDecl = cast<GenericTypeDecl>(getDecl(genericID));
-    typeOrOffset = UnboundGenericType::get(genericDecl, getType(parentID), ctx);
+    auto nominalOrError = getDeclChecked(genericID);
+    if (!nominalOrError)
+      return nominalOrError.takeError();
+    auto genericDecl = cast<GenericTypeDecl>(nominalOrError.get());
+
+    // FIXME: Check this?
+    auto parentTy = getType(parentID);
+
+    typeOrOffset = UnboundGenericType::get(genericDecl, parentTy, ctx);
     break;
   }
 
diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp
