IPC pickling optimization for render passes.

base/pickle.cc

Index: base/pickle.cc
diff --git a/base/pickle.cc b/base/pickle.cc
index af3191b9d86db2785fe375ecbcb045cf0a420870..4634561928de45f92ea2a127753ec6e089426da5 100644
--- a/base/pickle.cc
+++ b/base/pickle.cc
@@ -304,6 +304,16 @@ void Pickle::TrimWriteData(int new_length) {
   *cur_length = new_length;
 }
 
+void Pickle::Reserve(size_t additional_capacity) {
+  // write at a uint32-aligned offset from the beginning of the header

[jar (doing other things), 2013/10/25 01:54:18]

nit: Start with Upper case, end with a period.  (may as well fix line 318 too).

[danakj, 2013/10/25 18:40:47]

Done.

+  size_t offset = AlignInt(header_->payload_size, sizeof(uint32));
+
+  size_t new_size = offset + additional_capacity;
+  size_t needed_size = header_size_ + new_size;
+  if (needed_size > capacity_)
+    Resize(capacity_ * 2 + needed_size);

[jar (doing other things), 2013/10/25 01:54:18]

Why did you double the existing capacity?

Also...

Are you at all concerned about overflows, especially since this is called as a
public function, with presumably less care and scrutiny than other callers of
Resize()?

FWIW: Realloc already plays such games (exponential growth, by a factor of 2 as
I recall) when increasing sizes.  As a result, the number of calls to malloc (or
certainly TCMalloc) during these Resize() operations is limited to about
log2(final size).

Was this really a perf issue??  TCMalloc is fast... and log2 of most things are
small.


It strikes me that perchance the benchmarks are run with a very cold TCMalloc
cache, which needed to do page allocations afresh, and hence you're measuring
TCMalloc initialization time.

When you "skip" those intermediate allocations, you avoid having to warm up the
cache (via OS calls). 

Can you verify that re-running the tests more than once still shows a the same
perf numbers?  In a live Chromium app, the cache is generally quite well warmed
(sadly, having allocated lots of memory).

[danakj, 2013/10/25 18:40:47]

For the same reason we double it in BeginWrite(). Pickling N equally sized
things via reserve should not cause N reallocs to take place. This acts like
BeginWrite but is a hint that there are more writes coming after it that will
take up at least this much space. It does not try to be any more special than
that.
I'm no more concerned than we are in BeginWrite, and I'm not sure why I would
want to be? If we don't Reserve() then we're going to just call
WriteBytes()->BeginWrite() this many times anyways. I don't see the difference
here, am I overlooking something?
Right, but if we write 4000 objects with 10 fields each (and previously 2-16
elements in each field), we end up with a lot of calls to realloc anyways. Doing
it once is far superior to log_2(n) times, which was I guess around 14 times.
Yeh, you can see the bug for more information if you like. We were spending 3ms
just pickling a frame of data for the spaceport benchmark. This is a
pathalogical case, but one we care about being fast for.
The perf tests are here:
https://code.google.com/p/chromium/codesearch#chromium/src/content/common/cc_...

We write the ipc 10 times as a warm up, then write it 100 times and take the
mean of those 100 times as the result. Is that what you are thinking of?

+}
+
 char* Pickle::BeginWrite(size_t length) {
   // write at a uint32-aligned offset from the beginning of the header
   size_t offset = AlignInt(header_->payload_size, sizeof(uint32));