This CL optimizes C++11 range-based for loops where the variable is copied in each iteration but it would suffice to obtain it by const reference. This is only applied to loop variables of types that are expensive to copy which means they are not trivially copyable or have a non-trivial copy constructor or destructor.

To ensure that it is safe to replace the copy with a const reference the following heuristic is employed:
  The loop variable is const qualified.
  The loop variable is not const, but only const methods or operators are invoked on it, or it is used as const reference or value argument in constructors or function calls.

PiperOrigin-RevId: 305342272
Change-Id: I3e9955bf448d9670e41934437a90fa5c3b8c5b1c
This commit is contained in:
A. Unique TensorFlower 2020-04-07 14:33:47 -07:00 committed by TensorFlower Gardener
parent a21dd7457e
commit 91cb053af0
25 changed files with 39 additions and 38 deletions

View File

@ -321,7 +321,7 @@ xla::StatusOr<NodeDef> BuildXlaHostComputeNodeDef(
host_compute_builder.node_name());
// Copy all attributes.
for (auto attr : call_node->attrs()) {
for (const auto& attr : call_node->attrs()) {
host_compute_builder.Attr(attr.first, attr.second);
}
@ -346,7 +346,7 @@ xla::StatusOr<NodeDef> BuildXlaHostComputeNodeDef(
xla_token_input_nodes.emplace_back(kXlaTokenArgNodeName);
auto cluster_deps_it = cluster_deps.find(original_oc_name);
if (cluster_deps_it != cluster_deps.end()) {
for (auto dep : cluster_deps_it->second) {
for (const auto& dep : cluster_deps_it->second) {
xla_token_input_nodes.emplace_back(host_compute_node_name(dep));
}
}
@ -2359,7 +2359,7 @@ Status ExtractOutsideCompilationForFunction(
}
// For XlaHostCompute nodes with dependencies, add control edges between
// them so XlaCompiler can handle them in correct order.
for (auto iter : host_compute_nodes) {
for (const auto& iter : host_compute_nodes) {
Node* host_compute_node = iter.second;
std::vector<string> token_input_node_names;
TF_RETURN_IF_ERROR(GetNodeAttr(host_compute_node->def(),
@ -2479,7 +2479,7 @@ Status ExtractOutsideCompilation(
TF_RETURN_IF_ERROR(fld->RemoveFunction(host_graph_func_name));
for (auto shape_inference_graph_name : shape_inference_graphs) {
for (const auto& shape_inference_graph_name : shape_inference_graphs) {
TF_RETURN_IF_ERROR(RewriteShapeInferenceGraph(
shape_inference_graph_name, g, pivot_node, fld));
}

View File

@ -1161,7 +1161,7 @@ Status MarkForCompilationPassImpl::FindCompilationCandidates() {
std::vector<string> vall_ops = XlaOpRegistry::GetAllRegisteredOps();
absl::flat_hash_set<string> all_ops(vall_ops.begin(), vall_ops.end());
// Check that user's provided TF operation really exists.
for (auto s : whitelist) {
for (const auto& s : whitelist) {
if (!all_ops.contains(string(s))) {
return errors::InvalidArgument(
"The operation '", s,
@ -1475,7 +1475,7 @@ void MarkForCompilationPassImpl::VLogClusteringSummary() {
<< RatioToString(auto_clustering_info.clustered_node_count(),
graph_->num_nodes());
for (XlaAutoClusteringSummary::Cluster cluster :
for (const XlaAutoClusteringSummary::Cluster& cluster :
auto_clustering_info.clusters()) {
absl::string_view cluster_name = cluster.name();
int size = cluster.size();

View File

@ -493,7 +493,7 @@ std::pair<string, AttrValue> impl::AttrLiteralHelper(
const std::pair<string, absl::Span<const string>>& string_list_attr) {
AttrValue attr_value;
AttrValue::ListValue* list = attr_value.mutable_list();
for (string s : string_list_attr.second) {
for (const string& s : string_list_attr.second) {
list->add_s(s);
}
return {string_list_attr.first, attr_value};

View File

@ -641,7 +641,7 @@ Status ConvertAfterShapes(const ConversionParams& params) {
// Segment the graph into subgraphs that can be converted to TensorRT
segment::SegmentOptions segment_options;
// TODO(ben,jie,sami): exclude output nodes (DISCUSS IT)
for (auto node : *(params.output_names)) {
for (const auto& node : *(params.output_names)) {
segment_options.exclude_node_list.insert(node);
}
segment_options.minimum_segment_size = params.minimum_segment_size;

View File

@ -5738,7 +5738,7 @@ static void RegisterValidatableOpConverters(
{"FusedBatchNorm", "FusedBatchNormV2", "FusedBatchNormV3"}) {
(*registration)[normalization_op_type] = ConvertFusedBatchNorm;
}
for (auto unary_op_pair : *UnaryOperationMap()) {
for (const auto& unary_op_pair : *UnaryOperationMap()) {
(*registration)[unary_op_pair.first] = ConvertUnary;
}
for (auto reduce_op_type : {"Sum", "Prod", "Max", "Min", "Mean"}) {

View File

@ -93,7 +93,7 @@ void TRTOptimizationPass::PrintDebugInfo(grappler::Cluster* cluster,
const auto dev_names = cluster->GetDeviceNames();
if (!dev_names.empty()) {
LOG(INFO) << offset << " Device names:";
for (const auto s : dev_names) {
for (const auto& s : dev_names) {
LOG(INFO) << offset2 << s;
}
}
@ -101,7 +101,7 @@ void TRTOptimizationPass::PrintDebugInfo(grappler::Cluster* cluster,
auto status = cluster->GetPeakMemoryUsage(&peak_mem);
if (status == Status::OK()) {
LOG(INFO) << offset << "Peak Memory Usage :";
for (auto s : peak_mem) {
for (const auto& s : peak_mem) {
LOG(INFO) << offset2 << s.first << " = " << s.second;
}
}
@ -109,7 +109,7 @@ void TRTOptimizationPass::PrintDebugInfo(grappler::Cluster* cluster,
const auto dev_props = cluster->GetDevices();
if (!dev_props.empty()) {
LOG(INFO) << offset << "Device properties:";
for (auto k : dev_props) {
for (const auto& k : dev_props) {
LOG(INFO) << offset2 << k.first;
const auto& dt = k.second;
LOG(INFO) << offset3 << "type = " << dt.type();
@ -127,7 +127,7 @@ void TRTOptimizationPass::PrintDebugInfo(grappler::Cluster* cluster,
LOG(INFO) << offset3 << "bandwidth = " << dt.bandwidth();
if (dt.environment_size()) {
LOG(INFO) << offset3 << "environment :";
for (const auto e : dt.environment()) {
for (const auto& e : dt.environment()) {
LOG(INFO) << offset4 << e.first << " = " << e.second;
}
}

View File

@ -125,7 +125,7 @@ Status FunctionalizeControlFlowForFunction(
nodes_to_associated_functions.push_back({n, associated_functions});
}
}
for (auto iter : nodes_to_associated_functions) {
for (const auto& iter : nodes_to_associated_functions) {
Node* n = iter.first;
auto associated_functions = iter.second;
for (auto& associated_function : associated_functions) {

View File

@ -141,7 +141,7 @@ Status UpdateResourceUsageFromFunctionBodyAnalysis(
caller_source_to_path) {
std::unordered_map<std::string, Node*> node_name_index =
fbody.graph->BuildNodeNameIndex();
for (auto it : called_function_source_to_path) {
for (const auto& it : called_function_source_to_path) {
ResourceUsageAnalysis::NodeInfo src_node_info = it.first;
// If source is an _Arg, then the true source is actually corresponding
@ -309,7 +309,7 @@ Status AnalyzeResourceUsage(
}
}
for (auto it : user_to_source) {
for (const auto& it : user_to_source) {
(*source_to_path)[it.second].emplace(function_name, it.first->dst()->name(),
it.first->dst()->type_string());
}

View File

@ -621,7 +621,7 @@ Status RewriteAssociatedFunction(
NodeDebugInfo debug_info(*node);
NodeDefBuilder builder(node->name(), rewritten_function_name, fld,
&debug_info);
for (auto attr : node->attrs()) {
for (const auto& attr : node->attrs()) {
builder.Attr(attr.first, attr.second);
}
for (int i = 0; i < node->num_inputs(); i++) {
@ -695,7 +695,7 @@ Status CachedFunctionHandles::GetOrInstantiate(
Status CachedFunctionHandles::ReleaseAllHandles() {
Status result;
for (auto iter : handles_) {
for (const auto& iter : handles_) {
result.Update(flr_->ReleaseHandle(iter.second));
}
handles_.clear();

View File

@ -183,7 +183,7 @@ void BufRendezvous::LogContents() {
LOG(INFO) << strings::StrCat("BufRendezvous ",
strings::Hex(reinterpret_cast<uint64>(this)),
" step_id=", step_id_, " current contents:");
for (auto it : hook_table_) {
for (const auto& it : hook_table_) {
LOG(INFO) << it.first << ":" << it.second->DebugString();
}
}

View File

@ -1624,7 +1624,7 @@ Status DirectSession::CreateGraphs(
// Update our current state based on the execution_state's
// placements. If there are any mismatches for a node,
// we should fail, as this should never happen.
for (auto placement_pair : current_stateful_placements) {
for (const auto& placement_pair : current_stateful_placements) {
const string& node_name = placement_pair.first;
const string& placement = placement_pair.second;
auto iter = stateful_placements_.find(node_name);

View File

@ -305,7 +305,7 @@ void StepStatsCollector::BuildCostModel(
}
}
for (auto itr : device_map) {
for (const auto& itr : device_map) {
const StringPiece device = itr.first;
if (per_device_stats.find(device) == per_device_stats.end()) {
continue;

View File

@ -216,7 +216,7 @@ string SummarizeTensor(const TensorProto& tensor_proto) {
string SummarizeFunc(const NameAttrList& func) {
std::vector<string> entries;
for (auto p : func.attr()) {
for (const auto& p : func.attr()) {
entries.push_back(
strings::StrCat(p.first, "=", SummarizeAttrValue(p.second)));
}

View File

@ -102,7 +102,7 @@ string CollInstanceParams::ToString() const {
strings::StrAppend(&v, n, ", ");
}
strings::StrAppend(&v, "} num_devices_per_task={");
for (const auto dpt : num_devices_per_task) {
for (const auto& dpt : num_devices_per_task) {
strings::StrAppend(&v, dpt.first, ": ", dpt.second, ", ");
}
strings::StrAppend(&v, "}, collective_name=", impl_details.collective_name,

View File

@ -228,7 +228,7 @@ Status GraphDefBuilderWrapper::AddDataset(
opts.reset(new GraphDefBuilder::Options(
opts->WithAttr("output_types", dataset->output_dtypes())));
}
for (auto attr : attrs) {
for (const auto& attr : attrs) {
opts.reset(
new GraphDefBuilder::Options(opts->WithAttr(attr.first, attr.second)));
}

View File

@ -250,7 +250,7 @@ class GraphDefBuilderWrapper {
bool HasAttr(const string& op_type_name, const string& attr_name) const;
bool HasAttr(const OpDef* op_def, const string& attr_name) const {
for (auto attr : op_def->attr()) {
for (const auto& attr : op_def->attr()) {
if (attr.name() == attr_name) {
return true;
}

View File

@ -57,7 +57,7 @@ Status FunctionHandleCache::Instantiate(
Status FunctionHandleCache::Clear() {
mutex_lock l(mu_);
for (auto entry : handles_) {
for (const auto& entry : handles_) {
TF_RETURN_IF_ERROR(lib_->ReleaseHandle(entry.second));
}
handles_.clear();

View File

@ -53,7 +53,8 @@ NodeDef NDef(StringPiece name, StringPiece op, gtl::ArraySlice<string> inputs,
n.set_op(string(op));
for (const auto& in : inputs) n.add_input(in);
n.set_device(device);
for (auto na : attrs) n.mutable_attr()->insert({na.first, na.second.proto});
for (const auto& na : attrs)
n.mutable_attr()->insert({na.first, na.second.proto});
return n;
}

View File

@ -26,13 +26,13 @@ limitations under the License.
namespace tensorflow {
OpSegment::Item::~Item() {
for (auto kv : name_kernel) delete kv.second;
for (const auto& kv : name_kernel) delete kv.second;
}
OpSegment::OpSegment() {}
OpSegment::~OpSegment() {
for (auto kv : sessions_) delete kv.second;
for (const auto& kv : sessions_) delete kv.second;
}
Status OpSegment::FindOrCreate(const string& session_handle,

View File

@ -275,7 +275,7 @@ bool GenerateWrapperImports(CodeWriter* code_writer, const ModelInfo& model,
}
std::sort(imports.begin(), imports.end());
for (const auto target : imports) {
for (const auto& target : imports) {
code_writer->SetTokenValue("TARGET", target);
code_writer->Append("import {{TARGET}};");
}

View File

@ -42,7 +42,7 @@ void ResolveConflictedNamesByAddingIndex(std::vector<std::string>* names_ptr) {
names[i].append(std::to_string(indexes[names[i]]));
}
}
for (const auto it : first_appearance) {
for (const auto& it : first_appearance) {
const auto& name = it.first;
const auto i = it.second;
if (indexes[name] > 1) {
@ -148,11 +148,11 @@ void CodeGenerator::ResolveConflictedInputAndOutputNames(
std::unordered_set<std::string> io_conflict;
auto& input_names = *inputs;
auto& output_names = *outputs;
for (const auto input : input_names) {
for (const auto& input : input_names) {
if (io_conflict.find(input) != io_conflict.end()) {
continue;
}
for (const auto output : output_names) {
for (const auto& output : output_names) {
if (input == output) {
io_conflict.insert(input);
break;

View File

@ -197,7 +197,7 @@ class OpOptionData {
option_to_struct_["MirrorPadOptions"] = "TfLiteMirrorPaddingParams";
// Now for every op, try to find an option.
bool fatal = false;
for (auto op_name : ops_) {
for (const auto& op_name : ops_) {
bool found_option = false;
auto d = tflite::BuiltinOptionsTypeTable();
std::string collapsed_option_name_guess =

View File

@ -224,7 +224,7 @@ SubgraphWriter::ExportBuffers(flatbuffers::FlatBufferBuilder* fbb) {
flatbuffers::Offset<flatbuffers::Vector<flatbuffers::Offset<OperatorCode>>>
SubgraphWriter::CreateOpCodeTable(flatbuffers::FlatBufferBuilder* fbb) {
std::vector<flatbuffers::Offset<OperatorCode>> codes;
for (auto it : opcodes_) {
for (const auto& it : opcodes_) {
const char* custom_name = it.custom.empty() ? nullptr : it.custom.c_str();
codes.push_back(CreateOperatorCodeDirect(
*fbb, static_cast<BuiltinOperator>(it.builtin), custom_name));

View File

@ -218,8 +218,8 @@ Status MergeDuplicateNodes(const GraphDef& input_graph_def,
// duplicates and can be removed, unless they're stateful.
std::map<string, string> inputs_to_rename;
GraphDef merged_graph_def;
for (const std::pair<uint64, std::vector<const NodeDef*>> hashed_node_info :
hashed_nodes) {
for (const std::pair<const uint64, std::vector<const NodeDef*>>&
hashed_node_info : hashed_nodes) {
const std::vector<const NodeDef*>& hash_node_list =
hashed_node_info.second;
for (int i = 0; i < hash_node_list.size(); ++i) {

View File

@ -478,7 +478,7 @@ Status SparsifyGatherInternal(
}
// Add nodes with a reference count of 0 for deletion.
for (auto entry : refs) {
for (const auto& entry : refs) {
if (entry.second == 0) {
removed_node_names.push_back(entry.first);
}