[PATCH] D36235: [pdb/lld] Write a valid FPM

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/PDBFileBuilder.cpp:167-169
+  FpmStream =
+      WritableMappedBlockStream::createFpmStream(Layout, MsfBuffer, Allocator);
+  FpmWriter = BinaryStreamWriter(*FpmStream);
----------------
rnk wrote:
> zturner wrote:
> > zturner wrote:
> > > rnk wrote:
> > > > Why do you need to recreate the stream and writer?
> > > The stream doesn't have methods to update its length, so we need a new stream, and the writer doesn't have a method to set the stream to a new stream.
> > BTW, the cost of "making a new stream" is not high.  Nothing is really happening other than allocating a stream object and setting some of its fields.  So we're wasting one (small) heap allocated object, but there's no copying or anything going on.
> Are you sure? `createFpmStream` above seems to initialize the stream to 0xFF, so it's probably like a 4K memset at least.
Ahh, you are right.  I was confusing this code with code somewhere else.  This line shouldn't be here.  I used to have the initialization to 0xFF code here, so there was code to read the long version and then the short version.  Then I sunk all that code into the `createFpmStream` method and didn't clean up properly here.  Nice catch.


https://reviews.llvm.org/D36235