[kudu] 02/03: client: micro-optimize result batch cell getters by outlining cold path

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit d3b55212b9fdecc1b66c5f1163e65ec654a7bb77
Author: Todd Lipcon <to...@apache.org>
AuthorDate: Fri Dec 20 15:25:52 2019 -0800

    client: micro-optimize result batch cell getters by outlining cold path
    
    This improves performance in a couple ways:
    
    - by outlining the Substitute call and Status construction, the
      templated Get<TypeTraits> function becomes much smaller. This makes it
      get inlined into the GetInt8(), GetInt32(), etc outer functions. So,
      every call into those functions is just a single call instead of two
      calls in short succession. This makes branch prediction easier.
    
    - The addition of PREDICT_FALSE means the common path is now the
      "straight line" path which means that a tight loop calling these
      functions may be able to entirely fit within the micro-op cache, etc.
      Details here are a bit black magic but it's generally considered good
      hygiene for short loops to minimize the code footprint.
    
    - The outlining of the call also means the stack frame size is a bit
      smaller. The new code path has three "push" and "pop" instructions in
      the function intro/outro whereas the old one had 6.
    
    This improved TSBS perf about 10% for one of the workloads.
    
    Change-Id: I5838576b3e18cbe8e093bf1e9ce6418ce922de63
    Reviewed-on: http://gerrit.cloudera.org:8080/15503
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
    Tested-by: Todd Lipcon <to...@apache.org>
---
 src/kudu/client/scan_batch.cc | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/kudu/client/scan_batch.cc b/src/kudu/client/scan_batch.cc
index 324d01b..6f3c278 100644
--- a/src/kudu/client/scan_batch.cc
+++ b/src/kudu/client/scan_batch.cc
@@ -95,6 +95,12 @@ class RowCell {
   const int col_idx_;
 };
 
+Status BadTypeStatus(const char* provided_type_name, const ColumnSchema& col) {
+  return Status::InvalidArgument(
+      Substitute("invalid type $0 provided for column '$1' (expected $2)",
+                 provided_type_name, col.name(), col.type_info()->name()));
+}
+
 } // anonymous namespace
 
 bool KuduScanBatch::RowPtr::IsNull(int col_idx) const {
@@ -233,14 +239,13 @@ template<typename T>
 Status KuduScanBatch::RowPtr::Get(int col_idx, typename T::cpp_type* val) const {
   const ColumnSchema& col = schema_->column(col_idx);
   if (PREDICT_FALSE(col.type_info()->type() != T::type)) {
-    // TODO: at some point we could allow type coercion here.
-    return Status::InvalidArgument(
-        Substitute("invalid type $0 provided for column '$1' (expected $2)",
-                   T::name(),
-                   col.name(), col.type_info()->name()));
+    // TODO(todd): at some point we could allow type coercion here.
+    // Explicitly out-of-line the construction of this Status in order to
+    // keep the getter code footprint as small as possible.
+    return BadTypeStatus(T::name(), col);
   }
 
-  if (col.is_nullable() && IsNull(col_idx)) {
+  if (PREDICT_FALSE(col.is_nullable() && IsNull(col_idx))) {
     return Status::NotFound("column is NULL");
   }