[CodeView Type Merging] Avoid record deserialization when possible.

A profile shows the majority of time doing type merging is spent
deserializing records from sequences of bytes into friendly C++ structures
that we can easily access members of in order to find the type indices to
re-write.

Records are prefixed with their length, however, and most records have
type indices that appear at fixed offsets in the record. For these
records, we can save some cycles by just looking at the right place in the
byte sequence and re-writing the value, then skipping the record in the
type stream. This saves us from the costly deserialization of examining
every field, including potentially null terminated strings which are the
slowest, even though it was unnecessary to begin with.

In addition, we apply another optimization. Previously, after
deserializing a record and re-writing its type indices, we would
unconditionally re-serialize it in order to compute the hash of the
re-written record. This would result in an alloc and memcpy for every
record. If no type indices were re-written, however, this was an
unnecessary allocation. In this patch re-writing is made two phase. The
first phase discovers the indices that need to be rewritten and their new
values. This information is passed through to the de-duplication code,
which only copies and re-writes type indices in the serialized byte
sequence if at least one type index is different.

Some records have type indices which only appear after variable length
strings, or which have lists of type indices, or various other situations
that can make it tricky to make this optimization. While I'm not giving up
on optimizing these cases as well, for now we can get the easy cases out
of the way and lay the groundwork for more complicated cases later.

This patch yields another 50% speedup on top of the already large speedups
submitted over the past 2 days. In two tests I have run, I went from 9
seconds to 3 seconds, and from 16 seconds to 8 seconds.

Differential Revision: https://reviews.llvm.org/D33480

llvm-svn: 303914

10 files changed, 288 insertions, 148 deletions

diff --git a/llvm/include/llvm/DebugInfo/CodeView/CVRecord.h b/llvm/include/llvm/DebugInfo/CodeView/CVRecord.h
index 71ea82b6a9a..68ad0998220 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/CVRecord.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/CVRecord.h
@@ -14,6 +14,7 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/DebugInfo/CodeView/CodeViewError.h"
 #include "llvm/DebugInfo/CodeView/RecordSerialization.h"
+#include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/Support/BinaryStreamReader.h"
 #include "llvm/Support/BinaryStreamRef.h"
 #include "llvm/Support/Endian.h"
@@ -50,6 +51,13 @@ public:
   Optional<uint32_t> Hash;
 };
 
+template <typename Kind> struct RemappedRecord {
+  explicit RemappedRecord(const CVRecord<Kind> &R) : OriginalRecord(R) {}
+
+  CVRecord<Kind> OriginalRecord;
+  SmallVector<std::pair<uint32_t, TypeIndex>, 8> Mappings;
+};
+
 } // end namespace codeview
 
 template <typename Kind>
diff --git a/llvm/include/llvm/DebugInfo/CodeView/CVTypeVisitor.h b/llvm/include/llvm/DebugInfo/CodeView/CVTypeVisitor.h
index 4bc8fbefd5d..70ccc867cd3 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/CVTypeVisitor.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/CVTypeVisitor.h
@@ -46,6 +46,7 @@ Error visitMemberRecordStream(ArrayRef<uint8_t> FieldList,
                               TypeVisitorCallbacks &Callbacks);
 
 Error visitTypeStream(const CVTypeArray &Types, TypeVisitorCallbacks &Callbacks,
+                      VisitorDataSource Source = VDS_BytesPresent,
                       TypeServerHandler *TS = nullptr);
 Error visitTypeStream(CVTypeRange Types, TypeVisitorCallbacks &Callbacks,
                       TypeServerHandler *TS = nullptr);
diff --git a/llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h b/llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h
index 2142d4a2dec..a9c5cf42fc5 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/TypeDeserializer.h
@@ -40,6 +40,17 @@ class TypeDeserializer : public TypeVisitorCallbacks {
 public:
   TypeDeserializer() = default;
 
+  template <typename T> static Error deserializeAs(CVType &CVT, T &Record) {
+    MappingInfo I(CVT.content());
+    if (auto EC = I.Mapping.visitTypeBegin(CVT))
+      return EC;
+    if (auto EC = I.Mapping.visitKnownRecord(CVT, Record))
+      return EC;
+    if (auto EC = I.Mapping.visitTypeEnd(CVT))
+      return EC;
+    return Error::success();
+  }
+
   Error visitTypeBegin(CVType &Record) override {
     assert(!Mapping && "Already in a type mapping!");
     Mapping = llvm::make_unique<MappingInfo>(Record.content());
diff --git a/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h b/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
index 1f10872c876..fb71c72c80e 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/TypeRecord.h
@@ -35,6 +35,7 @@ using support::ulittle16_t;
 using support::ulittle32_t;
 
 typedef CVRecord<TypeLeafKind> CVType;
+typedef RemappedRecord<TypeLeafKind> RemappedType;
 
 struct CVMemberRecord {
   TypeLeafKind Kind;
diff --git a/llvm/include/llvm/DebugInfo/CodeView/TypeSerializer.h b/llvm/include/llvm/DebugInfo/CodeView/TypeSerializer.h
index deeb063c5df..f42fddd4f55 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/TypeSerializer.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/TypeSerializer.h
@@ -63,6 +63,15 @@ class TypeSerializer : public TypeVisitorCallbacks {
   /// Private type record hashing implementation details are handled here.
   std::unique_ptr<TypeHasher> Hasher;
 
+  /// Contains a list of all records indexed by TypeIndex.toArrayIndex().
+  SmallVector<ArrayRef<uint8_t>, 2> SeenRecords;
+
+  /// Temporary storage that we use to copy a record's data while re-writing
+  /// its type indices.
+  SmallVector<uint8_t, 256> RemapStorage;
+
+  TypeIndex nextTypeIndex() const;
+
   bool isInFieldList() const;
   MutableArrayRef<uint8_t> getCurrentSubRecordData();
   MutableArrayRef<uint8_t> getCurrentRecordData();
@@ -72,11 +81,12 @@ class TypeSerializer : public TypeVisitorCallbacks {
   addPadding(MutableArrayRef<uint8_t> Record);
 
 public:
-  explicit TypeSerializer(BumpPtrAllocator &Storage);
+  explicit TypeSerializer(BumpPtrAllocator &Storage, bool Hash = true);
   ~TypeSerializer();
 
   ArrayRef<ArrayRef<uint8_t>> records() const;
-  TypeIndex insertRecordBytes(ArrayRef<uint8_t> Record);
+  TypeIndex insertRecordBytes(ArrayRef<uint8_t> &Record);
+  TypeIndex insertRecord(const RemappedType &Record);
   Expected<TypeIndex> visitTypeEndGetIndex(CVType &Record);
 
   Error visitTypeBegin(CVType &Record) override;
diff --git a/llvm/include/llvm/DebugInfo/CodeView/TypeTableBuilder.h b/llvm/include/llvm/DebugInfo/CodeView/TypeTableBuilder.h
index c7d417063e4..8cc42feb87d 100644
--- a/llvm/include/llvm/DebugInfo/CodeView/TypeTableBuilder.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/TypeTableBuilder.h
@@ -68,6 +68,10 @@ public:
     return Serializer.insertRecordBytes(Record);
   }
 
+  TypeIndex writeSerializedRecord(const RemappedType &Record) {
+    return Serializer.insertRecord(Record);
+  }
+
   template <typename TFunc> void ForEachRecord(TFunc Func) {
     uint32_t Index = TypeIndex::FirstNonSimpleIndex;
 
@@ -88,7 +92,7 @@ class FieldListRecordBuilder {
 
 public:
   explicit FieldListRecordBuilder(TypeTableBuilder &TypeTable)
-      : TypeTable(TypeTable), TempSerializer(Allocator) {
+      : TypeTable(TypeTable), TempSerializer(Allocator, false) {
     Type.Type = TypeLeafKind::LF_FIELDLIST;
   }
 
diff --git a/llvm/lib/DebugInfo/CodeView/CVTypeVisitor.cpp b/llvm/lib/DebugInfo/CodeView/CVTypeVisitor.cpp
index f95c3e79388..705b548141b 100644
--- a/llvm/lib/DebugInfo/CodeView/CVTypeVisitor.cpp
+++ b/llvm/lib/DebugInfo/CodeView/CVTypeVisitor.cpp
@@ -45,24 +45,9 @@ static Error visitKnownMember(CVMemberRecord &Record,
 }
 
 static Expected<TypeServer2Record> deserializeTypeServerRecord(CVType &Record) {
-  class StealTypeServerVisitor : public TypeVisitorCallbacks {
-  public:
-    explicit StealTypeServerVisitor(TypeServer2Record &TR) : TR(TR) {}
-
-    Error visitKnownRecord(CVType &CVR, TypeServer2Record &Record) override {
-      TR = Record;
-      return Error::success();
-    }
-
-  private:
-    TypeServer2Record &TR;
-  };
-
   TypeServer2Record R(TypeRecordKind::TypeServer2);
-  StealTypeServerVisitor Thief(R);
-  if (auto EC = visitTypeRecord(Record, Thief))
+  if (auto EC = TypeDeserializer::deserializeAs(Record, R))
     return std::move(EC);
-
   return R;
 }
 
@@ -308,8 +293,9 @@ Error llvm::codeview::visitTypeRecord(CVType &Record,
 
 Error llvm::codeview::visitTypeStream(const CVTypeArray &Types,
                                       TypeVisitorCallbacks &Callbacks,
+                                      VisitorDataSource Source,
                                       TypeServerHandler *TS) {
-  VisitHelper V(Callbacks, VDS_BytesPresent);
+  VisitHelper V(Callbacks, Source);
   if (TS)
     V.Visitor.addTypeServerHandler(*TS);
   return V.Visitor.visitTypeStream(Types);
diff --git a/llvm/lib/DebugInfo/CodeView/TypeSerializer.cpp b/llvm/lib/DebugInfo/CodeView/TypeSerializer.cpp
index c667a73d086..4894277947b 100644
--- a/llvm/lib/DebugInfo/CodeView/TypeSerializer.cpp
+++ b/llvm/lib/DebugInfo/CodeView/TypeSerializer.cpp
@@ -75,29 +75,23 @@ private:
   /// entries are small and easy to rehash.
   DenseSet<HashedTypePtr> HashedRecords;
 
-  SmallVector<ArrayRef<uint8_t>, 2> SeenRecords;
-
-  TypeIndex NextTypeIndex = TypeIndex(TypeIndex::FirstNonSimpleIndex);
-
 public:
   TypeHasher(BumpPtrAllocator &RecordStorage) : RecordStorage(RecordStorage) {}
 
-  ArrayRef<ArrayRef<uint8_t>> records() const { return SeenRecords; }
-
   /// Takes the bytes of type record, inserts them into the hash table, saves
   /// them, and returns a pointer to an identical stable type record along with
   /// its type index in the destination stream.
-  TypeIndex getOrCreateRecord(ArrayRef<uint8_t> &Record);
+  TypeIndex getOrCreateRecord(ArrayRef<uint8_t> &Record, TypeIndex TI);
 };
 
-TypeIndex TypeHasher::getOrCreateRecord(ArrayRef<uint8_t> &Record) {
+TypeIndex TypeHasher::getOrCreateRecord(ArrayRef<uint8_t> &Record,
+                                        TypeIndex TI) {
   assert(Record.size() < UINT32_MAX && "Record too big");
   assert(Record.size() % 4 == 0 && "Record is not aligned to 4 bytes!");
 
   // Compute the hash up front so we can store it in the key.
   HashedType TempHashedType = {hash_value(Record), Record.data(),
-                               unsigned(Record.size()), NextTypeIndex};
-
+                               unsigned(Record.size()), TI};
   auto Result = HashedRecords.insert(HashedTypePtr(&TempHashedType));
   HashedType *&Hashed = Result.first->Ptr;
 
@@ -111,20 +105,15 @@ TypeIndex TypeHasher::getOrCreateRecord(ArrayRef<uint8_t> &Record) {
     memcpy(Stable, Record.data(), Record.size());
     Hashed->Data = Stable;
     assert(Hashed->Size == Record.size());
-
-    // This was a new record, so increment our next type index.
-    ++NextTypeIndex;
   }
 
   // Update the caller's copy of Record to point a stable copy.
   Record = ArrayRef<uint8_t>(Hashed->Data, Hashed->Size);
+  return Hashed->Index;
+}
 
-  if (Result.second) {
-    // FIXME: Can we record these in a more efficient way?
-    SeenRecords.push_back(Record);
-  }
-
-  return TypeIndex(Hashed->Index);
+TypeIndex TypeSerializer::nextTypeIndex() const {
+  return TypeIndex::fromArrayIndex(SeenRecords.size());
 }
 
 bool TypeSerializer::isInFieldList() const {
@@ -166,26 +155,68 @@ TypeSerializer::addPadding(MutableArrayRef<uint8_t> Record) {
   return MutableArrayRef<uint8_t>(Record.data(), Record.size() + N);
 }
 
-TypeSerializer::TypeSerializer(BumpPtrAllocator &Storage)
+TypeSerializer::TypeSerializer(BumpPtrAllocator &Storage, bool Hash)
     : RecordStorage(Storage), RecordBuffer(MaxRecordLength * 2),
       Stream(RecordBuffer, llvm::support::little), Writer(Stream),
-      Mapping(Writer), Hasher(make_unique<TypeHasher>(Storage)) {
+      Mapping(Writer) {
   // RecordBuffer needs to be able to hold enough data so that if we are 1
   // byte short of MaxRecordLen, and then we try to write MaxRecordLen bytes,
   // we won't overflow.
+  if (Hash)
+    Hasher = make_unique<TypeHasher>(Storage);
 }
 
 TypeSerializer::~TypeSerializer() = default;
 
 ArrayRef<ArrayRef<uint8_t>> TypeSerializer::records() const {
-  return Hasher->records();
+  return SeenRecords;
 }
 
-TypeIndex TypeSerializer::insertRecordBytes(ArrayRef<uint8_t> Record) {
+TypeIndex TypeSerializer::insertRecordBytes(ArrayRef<uint8_t> &Record) {
   assert(!TypeKind.hasValue() && "Already in a type mapping!");
   assert(Writer.getOffset() == 0 && "Stream has data already!");
 
-  return Hasher->getOrCreateRecord(Record);
+  if (Hasher) {
+    TypeIndex ActualTI = Hasher->getOrCreateRecord(Record, nextTypeIndex());
+    if (nextTypeIndex() == ActualTI)
+      SeenRecords.push_back(Record);
+    return ActualTI;
+  }
+
+  TypeIndex NewTI = nextTypeIndex();
+  uint8_t *Stable = RecordStorage.Allocate<uint8_t>(Record.size());
+  memcpy(Stable, Record.data(), Record.size());
+  Record = ArrayRef<uint8_t>(Stable, Record.size());
+  SeenRecords.push_back(Record);
+  return NewTI;
+}
+
+TypeIndex TypeSerializer::insertRecord(const RemappedType &Record) {
+  assert(!TypeKind.hasValue() && "Already in a type mapping!");
+  assert(Writer.getOffset() == 0 && "Stream has data already!");
+
+  TypeIndex TI;
+  ArrayRef<uint8_t> OriginalData = Record.OriginalRecord.RecordData;
+  if (Record.Mappings.empty()) {
+    // This record did not remap any type indices.  Just write it.
+    return insertRecordBytes(OriginalData);
+  }
+
+  // At least one type index was remapped.  Before we can hash it we have to
+  // copy the full record bytes, re-write each type index, then hash the copy.
+  // We do this in temporary storage since only the DenseMap can decide whether
+  // this record already exists, and if it does we don't want the memory to
+  // stick around.
+  RemapStorage.resize(OriginalData.size());
+  ::memcpy(&RemapStorage[0], OriginalData.data(), OriginalData.size());
+  uint8_t *ContentBegin = RemapStorage.data() + sizeof(RecordPrefix);
+  for (const auto &M : Record.Mappings) {
+    // First 4 bytes of every record are the record prefix, but the mapping
+    // offset is relative to the content which starts after.
+    *(TypeIndex *)(ContentBegin + M.first) = M.second;
+  }
+  auto RemapRef = makeArrayRef(RemapStorage);
+  return insertRecordBytes(RemapRef);
 }
 
 Error TypeSerializer::visitTypeBegin(CVType &Record) {
@@ -221,7 +252,12 @@ Expected<TypeIndex> TypeSerializer::visitTypeEndGetIndex(CVType &Record) {
 
   Record.Type = *TypeKind;
   Record.RecordData = ThisRecordData;
-  TypeIndex InsertedTypeIndex = Hasher->getOrCreateRecord(Record.RecordData);
+
+  // insertRecordBytes assumes we're not in a mapping, so do this first.
+  TypeKind.reset();
+  Writer.setOffset(0);
+
+  TypeIndex InsertedTypeIndex = insertRecordBytes(Record.RecordData);
 
   // Write out each additional segment in reverse order, and update each
   // record's continuation index to point to the previous one.
@@ -231,11 +267,9 @@ Expected<TypeIndex> TypeSerializer::visitTypeEndGetIndex(CVType &Record) {
         reinterpret_cast<support::ulittle32_t *>(CIBytes.data());
     assert(*CI == 0xB0C0B0C0 && "Invalid TypeIndex placeholder");
     *CI = InsertedTypeIndex.getIndex();
-    InsertedTypeIndex = Hasher->getOrCreateRecord(X);
+    InsertedTypeIndex = insertRecordBytes(X);
   }
 
-  TypeKind.reset();
-  Writer.setOffset(0);
   FieldListSegments.clear();
   CurrentSegment.SubRecords.clear();
 
diff --git a/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp b/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp
index 383799bca7e..0f8357ca80f 100644
--- a/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp
+++ b/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp
@@ -11,6 +11,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/DebugInfo/CodeView/CVTypeVisitor.h"
+#include "llvm/DebugInfo/CodeView/TypeDeserializer.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
 #include "llvm/DebugInfo/CodeView/TypeTableBuilder.h"
@@ -95,6 +96,33 @@ private:
   bool remapTypeIndex(TypeIndex &Idx);
   bool remapItemIndex(TypeIndex &Idx);
 
+  bool remapIndices(RemappedType &Record, ArrayRef<uint32_t> TidOffs,
+                    ArrayRef<uint32_t> IidOffs) {
+    auto OriginalData = Record.OriginalRecord.content();
+    bool Success = true;
+    for (auto Off : TidOffs) {
+      ArrayRef<uint8_t> Bytes = OriginalData.slice(Off, sizeof(TypeIndex));
+      TypeIndex OldTI(
+          *reinterpret_cast<const support::ulittle32_t *>(Bytes.data()));
+      TypeIndex NewTI = OldTI;
+      bool ThisSuccess = remapTypeIndex(NewTI);
+      if (ThisSuccess && NewTI != OldTI)
+        Record.Mappings.emplace_back(Off, NewTI);
+      Success &= ThisSuccess;
+    }
+    for (auto Off : IidOffs) {
+      ArrayRef<uint8_t> Bytes = OriginalData.slice(Off, sizeof(TypeIndex));
+      TypeIndex OldTI(
+          *reinterpret_cast<const support::ulittle32_t *>(Bytes.data()));
+      TypeIndex NewTI = OldTI;
+      bool ThisSuccess = remapItemIndex(NewTI);
+      if (ThisSuccess && NewTI != OldTI)
+        Record.Mappings.emplace_back(Off, NewTI);
+      Success &= ThisSuccess;
+    }
+    return Success;
+  }
+
   bool remapIndex(TypeIndex &Idx, ArrayRef<TypeIndex> Map);
 
   size_t slotForIndex(TypeIndex Idx) const {
@@ -107,23 +135,49 @@ private:
   }
 
   template <typename RecordType>
-  Error writeRecord(RecordType &R, bool RemapSuccess) {
+  Error writeKnownRecord(TypeTableBuilder &Dest, RecordType &R,
+                         bool RemapSuccess) {
     TypeIndex DestIdx = Untranslated;
     if (RemapSuccess)
-      DestIdx = DestTypeStream->writeKnownType(R);
+      DestIdx = Dest.writeKnownType(R);
     addMapping(DestIdx);
     return Error::success();
   }
 
   template <typename RecordType>
-  Error writeIdRecord(RecordType &R, bool RemapSuccess) {
+  Error writeKnownTypeRecord(RecordType &R, bool RemapSuccess) {
+    return writeKnownRecord(*DestTypeStream, R, RemapSuccess);
+  }
+
+  template <typename RecordType>
+  Error writeKnownIdRecord(RecordType &R, bool RemapSuccess) {
+    return writeKnownRecord(*DestIdStream, R, RemapSuccess);
+  }
+
+  Error writeRecord(TypeTableBuilder &Dest, const RemappedType &Record,
+                    bool RemapSuccess) {
     TypeIndex DestIdx = Untranslated;
     if (RemapSuccess)
-      DestIdx = DestIdStream->writeKnownType(R);
+      DestIdx = Dest.writeSerializedRecord(Record);
     addMapping(DestIdx);
     return Error::success();
   }
 
+  Error writeTypeRecord(const CVType &Record) {
+    TypeIndex DestIdx =
+        DestTypeStream->writeSerializedRecord(Record.RecordData);
+    addMapping(DestIdx);
+    return Error::success();
+  }
+
+  Error writeTypeRecord(const RemappedType &Record, bool RemapSuccess) {
+    return writeRecord(*DestTypeStream, Record, RemapSuccess);
+  }
+
+  Error writeIdRecord(const RemappedType &Record, bool RemapSuccess) {
+    return writeRecord(*DestIdStream, Record, RemapSuccess);
+  }
+
   template <typename RecordType>
   Error writeMember(RecordType &R, bool RemapSuccess) {
     if (RemapSuccess)
@@ -163,12 +217,10 @@ private:
 
 const TypeIndex TypeStreamMerger::Untranslated(SimpleTypeKind::NotTranslated);
 
-Error TypeStreamMerger::visitTypeBegin(CVRecord<TypeLeafKind> &Rec) {
-  return Error::success();
-}
+Error TypeStreamMerger::visitTypeBegin(CVType &Rec) { return Error::success(); }
 
-Error TypeStreamMerger::visitTypeEnd(CVRecord<TypeLeafKind> &Rec) {
-  CurIndex = TypeIndex(CurIndex.getIndex() + 1);
+Error TypeStreamMerger::visitTypeEnd(CVType &Rec) {
+  ++CurIndex;
   if (!IsSecondPass)
     assert(IndexMap.size() == slotForIndex(CurIndex) &&
            "visitKnownRecord should add one index map entry");
@@ -242,184 +294,206 @@ bool TypeStreamMerger::remapItemIndex(TypeIndex &Idx) {
 // Item records
 //----------------------------------------------------------------------------//
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, FuncIdRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, FuncIdRecord &R) {
   assert(DestIdStream);
-  bool Success = true;
-  Success &= remapItemIndex(R.ParentScope);
-  Success &= remapTypeIndex(R.FunctionType);
-  return writeIdRecord(R, Success);
+
+  RemappedType RR(CVR);
+  return writeIdRecord(RR, remapIndices(RR, {4}, {0}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, MemberFuncIdRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, MemberFuncIdRecord &R) {
   assert(DestIdStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.ClassType);
-  Success &= remapTypeIndex(R.FunctionType);
-  return writeIdRecord(R, Success);
+
+  RemappedType RR(CVR);
+  return writeIdRecord(RR, remapIndices(RR, {0, 4}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, StringIdRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, StringIdRecord &R) {
   assert(DestIdStream);
-  return writeIdRecord(R, remapItemIndex(R.Id));
+
+  RemappedType RR(CVR);
+  return writeIdRecord(RR, remapIndices(RR, {}, {0}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, StringListRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, StringListRecord &R) {
   assert(DestIdStream);
+
+  if (auto EC = TypeDeserializer::deserializeAs<StringListRecord>(CVR, R))
+    return EC;
   bool Success = true;
-  for (TypeIndex &Str : R.StringIndices)
-    Success &= remapItemIndex(Str);
-  return writeIdRecord(R, Success);
+
+  for (TypeIndex &Id : R.StringIndices)
+    Success &= remapItemIndex(Id);
+  return writeKnownIdRecord(R, Success);
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, BuildInfoRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, BuildInfoRecord &R) {
   assert(DestIdStream);
+
+  if (auto EC = TypeDeserializer::deserializeAs(CVR, R))
+    return EC;
+
   bool Success = true;
-  for (TypeIndex &Arg : R.ArgIndices)
-    Success &= remapItemIndex(Arg);
-  return writeIdRecord(R, Success);
+  for (TypeIndex &Str : R.ArgIndices)
+    Success &= remapItemIndex(Str);
+  return writeKnownIdRecord(R, Success);
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, UdtSourceLineRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, UdtSourceLineRecord &R) {
   assert(DestIdStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.UDT);
-  Success &= remapItemIndex(R.SourceFile);
+
+  RemappedType RR(CVR);
+
   // FIXME: Translate UdtSourceLineRecord into UdtModSourceLineRecords in the
   // IPI stream.
-  return writeIdRecord(R, Success);
+  return writeIdRecord(RR, remapIndices(RR, {0}, {4}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, UdtModSourceLineRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR,
+                                         UdtModSourceLineRecord &R) {
   assert(DestIdStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.UDT);
-  // UdtModSourceLine Source File Ids are offsets into the global string table.
+
+  RemappedType RR(CVR);
+
+  // UdtModSourceLine Source File Ids are offsets into the global string table,
+  // not type indices.
   // FIXME: We need to merge string table records for this to be valid.
-  // Success &= remapItemIndex(R.SourceFile);
-  return writeIdRecord(R, Success);
+  return writeIdRecord(RR, remapIndices(RR, {0}, {}));
 }
 
 //----------------------------------------------------------------------------//
 // Type records
 //----------------------------------------------------------------------------//
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, ModifierRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, ModifierRecord &R) {
   assert(DestTypeStream);
-  return writeRecord(R, remapTypeIndex(R.ModifiedType));
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {0}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, ProcedureRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, ProcedureRecord &R) {
   assert(DestTypeStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.ReturnType);
-  Success &= remapTypeIndex(R.ArgumentList);
-  return writeRecord(R, Success);
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {0, 8}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, MemberFunctionRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, MemberFunctionRecord &R) {
   assert(DestTypeStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.ReturnType);
-  Success &= remapTypeIndex(R.ClassType);
-  Success &= remapTypeIndex(R.ThisType);
-  Success &= remapTypeIndex(R.ArgumentList);
-  return writeRecord(R, Success);
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {0, 4, 8, 16}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &Type, ArgListRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, ArgListRecord &R) {
   assert(DestTypeStream);
+
+  if (auto EC = TypeDeserializer::deserializeAs(CVR, R))
+    return EC;
+
   bool Success = true;
   for (TypeIndex &Arg : R.ArgIndices)
     Success &= remapTypeIndex(Arg);
-  if (auto EC = writeRecord(R, Success))
-    return EC;
-  return Error::success();
+
+  return writeKnownTypeRecord(R, Success);
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, PointerRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, PointerRecord &R) {
   assert(DestTypeStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.ReferentType);
+
+  // Pointer records have a different number of TypeIndex mappings depending
+  // on whether or not it is a pointer to member.
+  if (auto EC = TypeDeserializer::deserializeAs(CVR, R))
+    return EC;
+
+  bool Success = remapTypeIndex(R.ReferentType);
   if (R.isPointerToMember())
     Success &= remapTypeIndex(R.MemberInfo->ContainingType);
-  return writeRecord(R, Success);
+  return writeKnownTypeRecord(R, Success);
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, ArrayRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, ArrayRecord &R) {
   assert(DestTypeStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.ElementType);
-  Success &= remapTypeIndex(R.IndexType);
-  return writeRecord(R, Success);
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {0, 4}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, ClassRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, ClassRecord &R) {
   assert(DestTypeStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.FieldList);
-  Success &= remapTypeIndex(R.DerivationList);
-  Success &= remapTypeIndex(R.VTableShape);
-  return writeRecord(R, Success);
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {4, 8, 12}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, UnionRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, UnionRecord &R) {
   assert(DestTypeStream);
-  return writeRecord(R, remapTypeIndex(R.FieldList));
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {4}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, EnumRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, EnumRecord &R) {
   assert(DestTypeStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.FieldList);
-  Success &= remapTypeIndex(R.UnderlyingType);
-  return writeRecord(R, Success);
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {4, 8}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, BitFieldRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, BitFieldRecord &R) {
   assert(DestTypeStream);
-  return writeRecord(R, remapTypeIndex(R.Type));
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {0}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, VFTableShapeRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, VFTableShapeRecord &R) {
   assert(DestTypeStream);
-  return writeRecord(R, true);
+
+  return writeTypeRecord(CVR);
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, TypeServer2Record &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, TypeServer2Record &R) {
   assert(DestTypeStream);
-  return writeRecord(R, true);
+
+  return writeTypeRecord(CVR);
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, LabelRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, LabelRecord &R) {
   assert(DestTypeStream);
-  return writeRecord(R, true);
+
+  return writeTypeRecord(CVR);
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, VFTableRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, VFTableRecord &R) {
   assert(DestTypeStream);
-  bool Success = true;
-  Success &= remapTypeIndex(R.CompleteClass);
-  Success &= remapTypeIndex(R.OverriddenVFTable);
-  return writeRecord(R, Success);
+
+  RemappedType RR(CVR);
+  return writeTypeRecord(RR, remapIndices(RR, {0, 4}, {}));
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &,
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR,
                                          MethodOverloadListRecord &R) {
   assert(DestTypeStream);
+
+  if (auto EC = TypeDeserializer::deserializeAs(CVR, R))
+    return EC;
+
   bool Success = true;
   for (OneMethodRecord &Meth : R.Methods)
     Success &= remapTypeIndex(Meth.Type);
-  return writeRecord(R, Success);
+  return writeKnownTypeRecord(R, Success);
 }
 
-Error TypeStreamMerger::visitKnownRecord(CVType &, FieldListRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &CVR, FieldListRecord &R) {
   assert(DestTypeStream);
   // Visit the members inside the field list.
   HadUntranslatedMember = false;
   FieldListBuilder = llvm::make_unique<FieldListRecordBuilder>(*DestTypeStream);
 
   FieldListBuilder->begin();
-  if (auto EC = codeview::visitMemberRecordStream(R.Data, *this))
+  if (auto EC = codeview::visitMemberRecordStream(CVR.content(), *this))
     return EC;
 
   // Write the record if we translated all field list members.
@@ -524,7 +598,11 @@ Error TypeStreamMerger::mergeTypesAndIds(TypeTableBuilder &DestIds,
 Error TypeStreamMerger::doit(const CVTypeArray &Types) {
   LastError = Error::success();
 
-  if (auto EC = codeview::visitTypeStream(Types, *this, Handler))
+  // We don't want to deserialize records.  I guess this flag is poorly named,
+  // but it really means "Don't deserialize records before switching on the
+  // concrete type.
+  if (auto EC =
+          codeview::visitTypeStream(Types, *this, VDS_BytesExternal, Handler))
     return EC;
 
   // If we found bad indices but no other errors, try doing another pass and see
@@ -540,7 +618,8 @@ Error TypeStreamMerger::doit(const CVTypeArray &Types) {
     NumBadIndices = 0;
     CurIndex = TypeIndex(TypeIndex::FirstNonSimpleIndex);
 
-    if (auto EC = codeview::visitTypeStream(Types, *this, Handler))
+    if (auto EC =
+            codeview::visitTypeStream(Types, *this, VDS_BytesExternal, Handler))
       return EC;
 
     assert(NumBadIndices <= BadIndicesRemaining &&
diff --git a/llvm/lib/DebugInfo/PDB/Native/PDBTypeServerHandler.cpp b/llvm/lib/DebugInfo/PDB/Native/PDBTypeServerHandler.cpp
index f00567db743..6ef987354aa 100644
--- a/llvm/lib/DebugInfo/PDB/Native/PDBTypeServerHandler.cpp
+++ b/llvm/lib/DebugInfo/PDB/Native/PDBTypeServerHandler.cpp
@@ -57,7 +57,13 @@ PDBTypeServerHandler::handleInternal(PDBFile &File,
   if (!ExpectedTpi)
     return ExpectedTpi.takeError();
 
-  if (auto EC = codeview::visitTypeStream(ExpectedTpi->typeArray(), Callbacks))
+  // For handling a type server, we should be using whatever the callback array
+  // was
+  // that is being used for the original file.  We shouldn't allow the visitor
+  // to
+  // arbitrarily stick a deserializer in there.
+  if (auto EC = codeview::visitTypeStream(ExpectedTpi->typeArray(), Callbacks,
+                                          VDS_BytesExternal))
     return std::move(EC);
 
   return true;