• brucedawson's avatar
    Revert of Adding StringPiece read/write support to pickle. (patchset #4... · d68c7ff8
    brucedawson authored
    Revert of Adding StringPiece read/write support to pickle. (patchset #4 id:60001 of https://codereview.chromium.org/927183002/)
    
    Reason for revert:
    Causing crashes on Linux. Investigating.
    Bug 464180.
    
    Original issue's description:
    > Adding StringPiece/StringPiece16 read/write support to pickle, and
    > update unit tests.
    >
    > The IPC perf tests do a lot of allocations which can, with large block
    > sizes, harm their performance. The high allocation counts also make
    > their performance very dependent on the quirks of the Windows heap, as
    > it applies undocumented heuristics to decide when to decommit memory
    > and when to save it for later.
    >
    > And, doing unnecessary allocations is generally always a bad thing.
    >
    > So, this change adds StringPiece support to PickleIterator (reading)
    > and Pickle (writing). The StringPiece function now handles all strings
    > when writing, but must be requested explicitly when reading.
    >
    > ipc_mojo_perftests does more allocations than ipc_perftests. This
    > removes one message-sized allocation from both tests.
    >
    > The unit tests were updated so that WriteString and WriteString16 are
    > exercised with both string objects and char/char16 arrays (no
    > allocations required!). Reading into StringPiece and StringPiece16 is
    > also tested and the tests were verified with:
    > out\Release\base_unittests --gtest_filter=PickleTest.*
    >
    > The main performance test command line used was:
    >
    > out\Release\ipc_mojo_perftests --gtest_filter=MojoChannelPerfTest.ChannelPingPong
    >
    > Typical test results on HP Z620 (Windows 8.1) with thread affinity and
    > high-performance power settings prior to this change:
    > IPC_Channel_Perf_50000x_12      1140.01 ms
    > IPC_Channel_Perf_50000x_144     1182.55 ms
    > IPC_Channel_Perf_50000x_1728    1266.42 ms
    > IPC_Channel_Perf_12000x_20736   1289.19 ms
    > IPC_Channel_Perf_1000x_248832   584.619 ms
    >
    > Typical test results with same settings but using base::StringPiece:
    > IPC_Channel_Perf_50000x_12      1123.33 ms
    > IPC_Channel_Perf_50000x_144     1164.53 ms
    > IPC_Channel_Perf_50000x_1728    1242.99 ms
    > IPC_Channel_Perf_12000x_20736   1186.84 ms
    > IPC_Channel_Perf_1000x_248832   496.469 ms
    >
    > Modest improvement with small buffers, but significant speedup with large buffers.
    >
    > Typical test results with large-blocks only prior to this change:
    > IPC_Channel_Perf_1000x_248832   1211.33 ms
    > IPC_Channel_Perf_1000x_248832   961.404 ms
    > IPC_Channel_Perf_1000x_248832   600.911 ms
    > IPC_Channel_Perf_1000x_248832   468.356 ms
    > IPC_Channel_Perf_1000x_248832   430.859 ms
    > IPC_Channel_Perf_1000x_248832   425.147 ms
    >
    > Typical test results with large-blocks only (base::StringPiece):
    > IPC_Channel_Perf_1000x_248832   909.571 ms
    > IPC_Channel_Perf_1000x_248832   731.435 ms
    > IPC_Channel_Perf_1000x_248832   493.697 ms
    > IPC_Channel_Perf_1000x_248832   417.966 ms
    > IPC_Channel_Perf_1000x_248832   397.377 ms
    > IPC_Channel_Perf_1000x_248832   397.725 ms
    >
    > Note that it takes a while for the Windows heap to 'realize' that it
    > should hang on to some of the large blocks which is why performance
    > increases over multiple runs.
    >
    > Chrome will not immediately be affected because StringPiece reading has
    > to be explicitly selected. Note that the effect on ipc_perftests is
    > more variable due to the odd Windows heap heuristics.
    >
    > Reliable results require setting the power plan to high-performance.
    > On Linux this is done with this command:
    > sudo cpupower frequency-set --governor performance
    >
    > The ipc_perftests command-line is:
    > out/Release/ipc_perftests --gtest_filter=IPCChannelPerfTest.ChannelPingPong
    >
    > Typical before/after Linux results for ipc_perftests are:
    > IPC_Channel_Perf_50000x_12      465.271 ms
    > IPC_Channel_Perf_50000x_144     480.224 ms
    > IPC_Channel_Perf_50000x_1728    510.871 ms
    > IPC_Channel_Perf_12000x_20736   318.016 ms
    > IPC_Channel_Perf_1000x_248832   309.325 ms
    >
    > IPC_Channel_Perf_50000x_12      459.245 ms
    > IPC_Channel_Perf_50000x_144     479.347 ms
    > IPC_Channel_Perf_50000x_1728    506.57  ms
    > IPC_Channel_Perf_12000x_20736   289.583 ms
    > IPC_Channel_Perf_1000x_248832   255.083 ms
    >
    > Before after Linux results for ipc_mojo_perftests:
    > IPC_Channel_Perf_50000x_12      670.727 ms
    > IPC_Channel_Perf_50000x_144     713.6   ms
    > IPC_Channel_Perf_50000x_1728    808.157 ms
    > IPC_Channel_Perf_12000x_20736   464.221 ms
    > IPC_Channel_Perf_1000x_248832   365.258 ms
    >
    > IPC_Channel_Perf_50000x_12      653.12  ms
    > IPC_Channel_Perf_50000x_144     697.418 ms
    > IPC_Channel_Perf_50000x_1728    772.575 ms
    > IPC_Channel_Perf_12000x_20736   446.315 ms
    > IPC_Channel_Perf_1000x_248832   348.38  ms
    >
    > So, consistent improvements on Linux.
    >
    > Committed: https://crrev.com/fcfde7d98209569fba81de4f1b26d0e26cd95848
    > Cr-Commit-Position: refs/heads/master@{#319128}
    
    TBR=thestig@chromium.org,cpu@chromium.org
    NOPRESUBMIT=true
    NOTREECHECKS=true
    NOTRY=true
    
    Review URL: https://codereview.chromium.org/981853002
    
    Cr-Commit-Position: refs/heads/master@{#319191}
    d68c7ff8
ipc_perftest_support.cc 11.2 KB