Todd Lipcon has submitted this change and it was merged. 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 Reviewed-on: http://gerrit.cloudera.org:8080/2532 Tested-by: Kudu Jenkins Reviewed-by: David Ribeiro Alves <[email protected]> --- M src/kudu/common/wire_protocol-test.cc 1 file changed, 17 insertions(+), 6 deletions(-) Approvals: David Ribeiro Alves: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/2532 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I22fc1b1a83ac57c819f320dce6ffc430ddf5662b Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]>
