You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2016/03/12 02:33:25 UTC

[kudu-CR] wire_protocol-test: make string data more realistic

Hello David Ribeiro Alves,

I'd like you to do a code review.  Please visit

    http://gerrit.cloudera.org:8080/2532

to review the following change.

Change subject: wire_protocol-test: make string data more realistic
......................................................................

wire_protocol-test: make string data more realistic

This test has had weirdly bimodal performance since it was introduced -- some
builds are consistently fast, and others are consistently slow, even if the two
builds had no changes to related code in between them.[1]

After much investigation, I determined that the "slow" builds spend a lot more
cycles waiting on L1D cache misses. It seems like the performance depends on
whether the constant string data happens to cross a page boundary. The string
data for the benchmark can move around in the constant section of the binary
(AKA ".rodata") based on other unrelated changes. Since the test was only
using a single string pointer, repeated thousands of times, the location
of that string was major factor in the benchmark result.

The "loads which split a page boundary are slow" issue is only vaguely
documented around the web or in Intel's optimization guides. I found one test
program[2] which demonstrates this effect. 16-byte loads which cross a page
boundary are ~62x slower than those which don't on my Haswell laptop.

In realistic scenarios, the string data would be a separate pointer for each
row, so if the occasional string crosses a page boundary, it won't be so
big a deal as this benchmark makes it out to be. So, this patch just
changes the benchmark to be more realistic and make new copies of the strings
for each row, at different memory locations.

Only time will tell, but my guess is that this will remove the weird
performance variance we've been seeing on this benchmark and make it easier to
evaluate actual changes to this code path.

[1] http://i.imgur.com/gZRBJ8S.png
[2] http://akuvian.org/src/pagesplit.c

Change-Id: I22fc1b1a83ac57c819f320dce6ffc430ddf5662b
---
M src/kudu/common/wire_protocol-test.cc
1 file changed, 17 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/2532/1
-- 
To view, visit http://gerrit.cloudera.org:8080/2532
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I22fc1b1a83ac57c819f320dce6ffc430ddf5662b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <da...@cloudera.com>