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 <[email protected]> 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 <[email protected]> Tested-by: Todd Lipcon <[email protected]> --- 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"); }
