[impala] 02/03: IMPALA-11223: Use unique id to create codegen instances

[db...@apache.org]

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

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

commit d98ab986a6a2218523ec147f70110300f019150b
Author: stiga-huang <hu...@gmail.com>
AuthorDate: Thu Feb 16 20:30:50 2023 +0800

    IMPALA-11223: Use unique id to create codegen instances
    
    When startup flag asm_module_dir is set, impalad will dump the codegen
    disassembly to files under that folder. The file name is "id.asm" in
    which "id" is the codegen instance id. Before IMPALA-4080 (f2837e9), we
    used fragment instance id as the codegen id. After that, since codegen
    is done in fragment level (shared by fragment instances), we use query
    id instead. This introduces conflicts between different fragments. The
    asm files will be overwritten.
    
    The same conflict happens in dumping IR modules (when unopt_module_dir
    or opt_module_dir is set).
    
    This changes the codegen instance id to be "QueryID_FragmentName_PID".
    The PID suffix is needed since we usually have several impalads running
    together on our dev box.
    
    Also adds logs when IR or disassembly are dumped to files. It helps to
    know which instance performs the codegen.
    
    Tests:
     - Manually verified the asm file names are expected.
    
    Change-Id: I7672906365c916bbe750eeb9906cab38573e6c31
    Reviewed-on: http://gerrit.cloudera.org:8080/19505
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/codegen/codegen-symbol-emitter.cc | 5 ++++-
 be/src/codegen/llvm-codegen.cc           | 2 ++
 be/src/runtime/fragment-state.cc         | 6 +++++-
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/be/src/codegen/codegen-symbol-emitter.cc b/be/src/codegen/codegen-symbol-emitter.cc
index 992493bff..ce8d3c547 100644
--- a/be/src/codegen/codegen-symbol-emitter.cc
+++ b/be/src/codegen/codegen-symbol-emitter.cc
@@ -74,7 +74,10 @@ void CodegenSymbolEmitter::NotifyObjectEmitted(const llvm::object::ObjectFile& o
     ProcessSymbol(&dwarf_ctx, pair.first, pair.second, &perf_map_entries, asm_file);
   }
 
-  if (asm_file.is_open()) asm_file.close();
+  if (asm_file.is_open()) {
+    asm_file.close();
+    LOG(INFO) << "Saved disassembly to " << asm_path_;
+  }
 
   ofstream perf_map_file;
   if (emit_perf_map_) {
diff --git a/be/src/codegen/llvm-codegen.cc b/be/src/codegen/llvm-codegen.cc
index d31eee772..1b3b835ff 100644
--- a/be/src/codegen/llvm-codegen.cc
+++ b/be/src/codegen/llvm-codegen.cc
@@ -1262,6 +1262,7 @@ Status LlvmCodeGen::FinalizeModule() {
     } else {
       f << GetIR(true);
       f.close();
+      LOG(INFO) << "Saved unoptimized IR to " << path;
     }
   }
 
@@ -1328,6 +1329,7 @@ Status LlvmCodeGen::FinalizeModule() {
     } else {
       f << GetIR(true);
       f.close();
+      LOG(INFO) << "Saved optimized IR to " << path;
     }
   }
 
diff --git a/be/src/runtime/fragment-state.cc b/be/src/runtime/fragment-state.cc
index 60a275176..a9e8d23b5 100644
--- a/be/src/runtime/fragment-state.cc
+++ b/be/src/runtime/fragment-state.cc
@@ -155,8 +155,12 @@ void FragmentState::ReleaseResources() {
 
 Status FragmentState::CreateCodegen() {
   if (codegen_.get() != NULL) return Status::OK();
+  // Create with an id of "QueryId_FragmentName_PID" to avoid conflicts between fragments.
+  // The id is used when dumping IR modules or codegen disassembly.
   RETURN_IF_ERROR(LlvmCodeGen::CreateImpalaCodegen(
-      this, query_mem_tracker(), PrintId(query_id()), &codegen_));
+      this, query_mem_tracker(),
+      Substitute("$0_$1_$2", PrintId(query_id()), fragment_.display_name, getpid()),
+      &codegen_));
   codegen_->EnableOptimizations(true);
   runtime_profile_->AddChild(codegen_->runtime_profile());
   return Status::OK();