Repository: incubator-impala Updated Branches: refs/heads/master 6fc399ebc -> be6a3bc1a
Remove dead and untested code Remove some code that the code coverage build revealed was untested. It is all dead, aside from SetErrorMsg(), which only had one callsite. Change-Id: I49c27cbfef03ef97befa9a607b3d8d7ac6e22a43 Reviewed-on: http://gerrit.cloudera.org:8080/3989 Reviewed-by: Dan Hecht <[email protected]> Tested-by: Internal Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/5ec76c61 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/5ec76c61 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/5ec76c61 Branch: refs/heads/master Commit: 5ec76c618f50405c154ebe5587b6be5d4f8d234d Parents: 6fc399e Author: Tim Armstrong <[email protected]> Authored: Mon Aug 15 09:26:36 2016 -0700 Committer: Internal Jenkins <[email protected]> Committed: Tue Aug 16 01:50:50 2016 +0000 ---------------------------------------------------------------------- be/src/common/status.h | 9 ---- be/src/exprs/scalar-fn-call.cc | 5 +- be/src/runtime/coordinator.cc | 94 ---------------------------------- be/src/runtime/descriptors.cc | 45 ---------------- be/src/runtime/descriptors.h | 7 --- be/src/runtime/raw-value.cc | 55 -------------------- be/src/runtime/raw-value.h | 5 -- be/src/service/query-exec-state.h | 2 - 8 files changed, 2 insertions(+), 220 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5ec76c61/be/src/common/status.h ---------------------------------------------------------------------- diff --git a/be/src/common/status.h b/be/src/common/status.h index ef94540..35a9a94 100644 --- a/be/src/common/status.h +++ b/be/src/common/status.h @@ -199,15 +199,6 @@ class Status { return *msg_; } - /// Sets the ErrorMessage on the detail of the status. Calling this method is only valid - /// if an error was reported. - /// TODO: deprecate, error should be immutable - void SetErrorMsg(const ErrorMsg& m) { - DCHECK(msg_ != NULL); - delete msg_; - msg_ = new ErrorMsg(m); - } - /// Add a detail string. Calling this method is only defined on a non-OK message void AddDetail(const std::string& msg); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5ec76c61/be/src/exprs/scalar-fn-call.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/scalar-fn-call.cc b/be/src/exprs/scalar-fn-call.cc index a43b6c9..b2fce53 100644 --- a/be/src/exprs/scalar-fn-call.cc +++ b/be/src/exprs/scalar-fn-call.cc @@ -121,9 +121,8 @@ Status ScalarFnCall::Prepare(RuntimeState* state, const RowDescriptor& desc, if (!status.ok()) { if (fn_.binary_type == TFunctionBinaryType::BUILTIN) { // Builtins symbols should exist unless there is a version mismatch. - status.SetErrorMsg(ErrorMsg(TErrorCode::MISSING_BUILTIN, - fn_.name.function_name, fn_.scalar_fn.symbol)); - return status; + return Status(TErrorCode::MISSING_BUILTIN, fn_.name.function_name, + fn_.scalar_fn.symbol); } else { DCHECK_EQ(fn_.binary_type, TFunctionBinaryType::NATIVE); return Status(Substitute("Problem loading UDF '$0':\n$1", http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5ec76c61/be/src/runtime/coordinator.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/coordinator.cc b/be/src/runtime/coordinator.cc index ad2fd9b..66c792c 100644 --- a/be/src/runtime/coordinator.cc +++ b/be/src/runtime/coordinator.cc @@ -122,7 +122,6 @@ struct DebugOptions { /// Execution state of a particular fragment instance. /// /// Concurrent accesses: -/// - GetNodeThroughput() called when coordinator's profile is printed /// - updates through UpdateFragmentExecStatus() class Coordinator::FragmentInstanceState { public: @@ -156,14 +155,6 @@ class Coordinator::FragmentInstanceState { /// Computes sum of split sizes of leftmost scan. void ComputeTotalSplitSize(const PerNodeScanRanges& per_node_scan_ranges); - /// Return value of throughput counter for given plan_node_id, or 0 if that node doesn't - /// exist. Thread-safe, and takes lock() internally. - int64_t GetNodeThroughput(int plan_node_id); - - /// Return number of completed scan ranges for plan_node_id, or 0 if that node doesn't - /// exist. Thread-safe, and takes lock() internally. - int64_t GetNumScanRangesCompleted(int plan_node_id); - /// Updates the total number of scan ranges complete for this fragment. Returns the /// delta since the last time this was called. Not thread-safe without lock() being /// acquired by the caller. @@ -271,34 +262,6 @@ void Coordinator::FragmentInstanceState::ComputeTotalSplitSize( } } -int64_t Coordinator::FragmentInstanceState::GetNodeThroughput(int plan_node_id) { - RuntimeProfile::Counter* counter = NULL; - { - lock_guard<mutex> l(lock_); - CounterMap& throughput_counters = aggregate_counters_.throughput_counters; - CounterMap::iterator i = throughput_counters.find(plan_node_id); - if (i == throughput_counters.end()) return 0; - counter = i->second; - } - DCHECK(counter != NULL); - // make sure not to hold lock when calling value() to avoid potential deadlocks - return counter->value(); -} - -int64_t Coordinator::FragmentInstanceState::GetNumScanRangesCompleted(int plan_node_id) { - RuntimeProfile::Counter* counter = NULL; - { - lock_guard<mutex> l(lock_); - CounterMap& ranges_complete = aggregate_counters_.scan_ranges_complete_counters; - CounterMap::iterator i = ranges_complete.find(plan_node_id); - if (i == ranges_complete.end()) return 0; - counter = i->second; - } - DCHECK(counter != NULL); - // make sure not to hold lock when calling value() to avoid potential deadlocks - return counter->value(); -} - int64_t Coordinator::FragmentInstanceState::UpdateNumScanRangesCompleted() { int64_t total = 0; CounterMap& complete = aggregate_counters_.scan_ranges_complete_counters; @@ -1304,63 +1267,6 @@ void Coordinator::CollectScanNodeCounters(RuntimeProfile* profile, } } -void Coordinator::CreateAggregateCounters( - const vector<TPlanFragment>& fragments) { - for (const TPlanFragment& fragment: fragments) { - if (!fragment.__isset.plan) continue; - const vector<TPlanNode>& nodes = fragment.plan.nodes; - for (const TPlanNode& node: nodes) { - if (node.node_type != TPlanNodeType::HDFS_SCAN_NODE - && node.node_type != TPlanNodeType::HBASE_SCAN_NODE) { - continue; - } - - stringstream s; - s << PrintPlanNodeType(node.node_type) << " (id=" - << node.node_id << ") Throughput"; - query_profile_->AddDerivedCounter(s.str(), TUnit::BYTES_PER_SECOND, - bind<int64_t>(mem_fn(&Coordinator::ComputeTotalThroughput), - this, node.node_id)); - s.str(""); - s << PrintPlanNodeType(node.node_type) << " (id=" - << node.node_id << ") Completed scan ranges"; - query_profile_->AddDerivedCounter(s.str(), TUnit::UNIT, - bind<int64_t>(mem_fn(&Coordinator::ComputeTotalScanRangesComplete), - this, node.node_id)); - } - } -} - -int64_t Coordinator::ComputeTotalThroughput(int node_id) { - int64_t value = 0; - for (int i = 0; i < fragment_instance_states_.size(); ++i) { - FragmentInstanceState* exec_state = fragment_instance_states_[i]; - value += exec_state->GetNodeThroughput(node_id); - } - // Add up the local fragment throughput counter - CounterMap& throughput_counters = coordinator_counters_.throughput_counters; - CounterMap::iterator it = throughput_counters.find(node_id); - if (it != throughput_counters.end()) { - value += it->second->value(); - } - return value; -} - -int64_t Coordinator::ComputeTotalScanRangesComplete(int node_id) { - int64_t value = 0; - for (int i = 0; i < fragment_instance_states_.size(); ++i) { - FragmentInstanceState* exec_state = fragment_instance_states_[i]; - value += exec_state->GetNumScanRangesCompleted(node_id); - } - // Add up the local fragment throughput counter - CounterMap& scan_ranges_complete = coordinator_counters_.scan_ranges_complete_counters; - CounterMap::iterator it = scan_ranges_complete.find(node_id); - if (it != scan_ranges_complete.end()) { - value += it->second->value(); - } - return value; -} - void Coordinator::ExecRemoteFragment(const FragmentExecParams* fragment_exec_params, const TPlanFragment* plan_fragment, DebugOptions* debug_options, QuerySchedule* schedule, int instance_state_idx, int fragment_idx, http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5ec76c61/be/src/runtime/descriptors.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/descriptors.cc b/be/src/runtime/descriptors.cc index 93a035c..548370a 100644 --- a/be/src/runtime/descriptors.cc +++ b/be/src/runtime/descriptors.cc @@ -393,16 +393,6 @@ RowDescriptor::RowDescriptor(const RowDescriptor& lhs_row_desc, InitHasVarlenSlots(); } -RowDescriptor::RowDescriptor(const vector<TupleDescriptor*>& tuple_descs, - const vector<bool>& nullable_tuples) - : tuple_desc_map_(tuple_descs), - tuple_idx_nullable_map_(nullable_tuples) { - DCHECK_EQ(nullable_tuples.size(), tuple_descs.size()); - DCHECK_GT(tuple_descs.size(), 0); - InitTupleIdxMap(); - InitHasVarlenSlots(); -} - RowDescriptor::RowDescriptor(TupleDescriptor* tuple_desc, bool is_nullable) : tuple_desc_map_(1, tuple_desc), tuple_idx_nullable_map_(1, is_nullable) { @@ -581,41 +571,6 @@ void DescriptorTbl::GetTupleDescs(vector<TupleDescriptor*>* descs) const { } } -// Generate function to check if a slot is null. The resulting IR looks like: -// (in this case the tuple contains only a nullable double) -// define i1 @IsNull({ i8, double }* %tuple) { -// entry: -// %null_byte_ptr = getelementptr inbounds { i8, double }* %tuple, i32 0, i32 0 -// %null_byte = load i8* %null_byte_ptr -// %null_mask = and i8 %null_byte, 1 -// %is_null = icmp ne i8 %null_mask, 0 -// ret i1 %is_null -// } -Function* SlotDescriptor::GetIsNullFn(LlvmCodeGen* codegen) const { - if (is_null_fn_ != NULL) return is_null_fn_; - StructType* tuple_type = parent()->GetLlvmStruct(codegen); - PointerType* tuple_ptr_type = tuple_type->getPointerTo(); - LlvmCodeGen::FnPrototype prototype(codegen, "IsNull", codegen->GetType(TYPE_BOOLEAN)); - prototype.AddArgument(LlvmCodeGen::NamedVariable("tuple", tuple_ptr_type)); - - Value* mask = codegen->GetIntConstant(TYPE_TINYINT, null_indicator_offset_.bit_mask); - Value* zero = codegen->GetIntConstant(TYPE_TINYINT, 0); - int byte_offset = null_indicator_offset_.byte_offset; - - LlvmCodeGen::LlvmBuilder builder(codegen->context()); - Value* tuple_ptr; - Function* fn = prototype.GeneratePrototype(&builder, &tuple_ptr); - - Value* null_byte_ptr = builder.CreateStructGEP(NULL, tuple_ptr, byte_offset, - "null_byte_ptr"); - Value* null_byte = builder.CreateLoad(null_byte_ptr, "null_byte"); - Value* null_mask = builder.CreateAnd(null_byte, mask, "null_mask"); - Value* is_null = builder.CreateICmpNE(null_mask, zero, "is_null"); - builder.CreateRet(is_null); - - return is_null_fn_ = codegen->FinalizeFunction(fn); -} - // Generate function to set a slot to be null or not-null. The resulting IR // for SetNotNull looks like: // (in this case the tuple contains only a nullable double) http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5ec76c61/be/src/runtime/descriptors.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/descriptors.h b/be/src/runtime/descriptors.h index ff3a53a..10c2ff3 100644 --- a/be/src/runtime/descriptors.h +++ b/be/src/runtime/descriptors.h @@ -133,10 +133,6 @@ class SlotDescriptor { std::string DebugString() const; - /// Codegen for: bool IsNull(Tuple* tuple) - /// The codegen'd IR function is cached. - llvm::Function* GetIsNullFn(LlvmCodeGen*) const; - /// Codegen for: void SetNull(Tuple* tuple) / void SetNotNull(Tuple* tuple) /// The codegen'd IR function is cached. llvm::Function* GetUpdateNullFn(LlvmCodeGen*, bool set_null) const; @@ -482,9 +478,6 @@ class RowDescriptor { /// c'tor for a row assembled from two rows RowDescriptor(const RowDescriptor& lhs_row_desc, const RowDescriptor& rhs_row_desc); - RowDescriptor(const std::vector<TupleDescriptor*>& tuple_descs, - const std::vector<bool>& nullable_tuples); - RowDescriptor(TupleDescriptor* tuple_desc, bool is_nullable); /// dummy descriptor, needed for the JNI EvalPredicate() function http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5ec76c61/be/src/runtime/raw-value.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/raw-value.cc b/be/src/runtime/raw-value.cc index 296dead..7247b8e 100644 --- a/be/src/runtime/raw-value.cc +++ b/be/src/runtime/raw-value.cc @@ -181,61 +181,6 @@ void RawValue::Write(const void* value, void* dst, const ColumnType& type, } } -// TODO: can we remove some of this code duplication? Templated allocator? -void RawValue::Write(const void* value, const ColumnType& type, - void* dst, uint8_t** buf) { - DCHECK(value != NULL); - switch (type.type) { - case TYPE_BOOLEAN: - *reinterpret_cast<bool*>(dst) = *reinterpret_cast<const bool*>(value); - break; - case TYPE_TINYINT: - *reinterpret_cast<int8_t*>(dst) = *reinterpret_cast<const int8_t*>(value); - break; - case TYPE_SMALLINT: - *reinterpret_cast<int16_t*>(dst) = *reinterpret_cast<const int16_t*>(value); - break; - case TYPE_INT: - *reinterpret_cast<int32_t*>(dst) = *reinterpret_cast<const int32_t*>(value); - break; - case TYPE_BIGINT: - *reinterpret_cast<int64_t*>(dst) = *reinterpret_cast<const int64_t*>(value); - break; - case TYPE_FLOAT: - *reinterpret_cast<float*>(dst) = *reinterpret_cast<const float*>(value); - break; - case TYPE_DOUBLE: - *reinterpret_cast<double*>(dst) = *reinterpret_cast<const double*>(value); - break; - case TYPE_TIMESTAMP: - *reinterpret_cast<TimestampValue*>(dst) = - *reinterpret_cast<const TimestampValue*>(value); - break; - case TYPE_STRING: - case TYPE_VARCHAR: - case TYPE_CHAR: { - DCHECK(buf != NULL); - if (!type.IsVarLenStringType()) { - DCHECK_EQ(type.type, TYPE_CHAR); - memcpy(dst, value, type.len); - break; - } - const StringValue* src = reinterpret_cast<const StringValue*>(value); - StringValue* dest = reinterpret_cast<StringValue*>(dst); - dest->len = src->len; - dest->ptr = reinterpret_cast<char*>(*buf); - memcpy(dest->ptr, src->ptr, dest->len); - *buf += dest->len; - break; - } - case TYPE_DECIMAL: - memcpy(dst, value, type.GetByteSize()); - break; - default: - DCHECK(false) << "RawValue::Write(): bad type: " << type.DebugString(); - } -} - void RawValue::Write(const void* value, Tuple* tuple, const SlotDescriptor* slot_desc, MemPool* pool) { if (value == NULL) { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5ec76c61/be/src/runtime/raw-value.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/raw-value.h b/be/src/runtime/raw-value.h index 6b4c084..bc76b2c 100644 --- a/be/src/runtime/raw-value.h +++ b/be/src/runtime/raw-value.h @@ -92,11 +92,6 @@ class RawValue { /// src must be non-NULL. static void Write(const void* src, void* dst, const ColumnType& type, MemPool* pool); - /// Writes 'src' into 'dst' for type. - /// String values are copied into *buffer and *buffer is updated by the length. *buf - /// must be preallocated to be large enough. - static void Write(const void* src, const ColumnType& type, void* dst, uint8_t** buf); - /// Returns true if v1 == v2. /// This is more performant than Compare() == 0 for string equality, mostly because of /// the length comparison check. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5ec76c61/be/src/service/query-exec-state.h ---------------------------------------------------------------------- diff --git a/be/src/service/query-exec-state.h b/be/src/service/query-exec-state.h index 209631a..feb8afe 100644 --- a/be/src/service/query-exec-state.h +++ b/be/src/service/query-exec-state.h @@ -256,8 +256,6 @@ class ImpalaServer::QueryExecState { /// Max size of the result_cache_ in number of rows. A value <= 0 means no caching. int64_t result_cache_max_size_; - /// local runtime_state_ in case we don't have a coord_ - boost::scoped_ptr<RuntimeState> local_runtime_state_; ObjectPool profile_pool_; /// The QueryExecState builds three separate profiles.
