From c5d94017569713c12a7e3d260cb4cd0a02b26372 Mon Sep 17 00:00:00 2001
From: Nicholas Nethercote <nnethercote@mozilla.com>
Date: Mon, 6 May 2019 11:47:03 +1000
Subject: [PATCH] Avoid symbol interning in `file_metadata`.

This commit changes `created_files` so it uses strings directly as keys,
rather than symbols derived from the strings. This avoids the cost of
having to do the hash table lookups to produce the symbols from the
strings.

The commit also uses `entry` to avoid doing a repeated hash table lookup
(`get` + `insert`).

Note that PR #60467 improved this code somewhat; this is a further
improvement.
---
 .../debuginfo/metadata.rs                     | 64 +++++++++----------
 src/librustc_codegen_llvm/debuginfo/mod.rs    |  4 +-
 2 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs
index 9bdce37b8baec..e7f54f6609d29 100644
--- a/src/librustc_codegen_llvm/debuginfo/metadata.rs
+++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs
@@ -38,6 +38,7 @@ use rustc_data_structures::small_c_str::SmallCStr;
 use rustc_target::abi::HasDataLayout;
 
 use libc::{c_uint, c_longlong};
+use std::collections::hash_map::Entry;
 use std::ffi::CString;
 use std::fmt::{self, Write};
 use std::hash::{Hash, Hasher};
@@ -45,7 +46,7 @@ use std::iter;
 use std::ptr;
 use std::path::{Path, PathBuf};
 use syntax::ast;
-use syntax::symbol::{Interner, InternedString, Symbol};
+use syntax::symbol::{Interner, InternedString};
 use syntax_pos::{self, Span, FileName};
 
 impl PartialEq for llvm::Metadata {
@@ -787,49 +788,48 @@ pub fn file_metadata(cx: &CodegenCx<'ll, '_>,
            file_name,
            defining_crate);
 
-    let file_name = &file_name.to_string();
-    let file_name_symbol = Symbol::intern(file_name);
-    if defining_crate == LOCAL_CRATE {
-        let directory = &cx.sess().working_dir.0.to_string_lossy();
-        file_metadata_raw(cx, file_name, Some(file_name_symbol),
-                          directory, Some(Symbol::intern(directory)))
+    let file_name = Some(file_name.to_string());
+    let directory = if defining_crate == LOCAL_CRATE {
+        Some(cx.sess().working_dir.0.to_string_lossy().to_string())
     } else {
         // If the path comes from an upstream crate we assume it has been made
         // independent of the compiler's working directory one way or another.
-        file_metadata_raw(cx, file_name, Some(file_name_symbol), "", None)
-    }
+        None
+    };
+    file_metadata_raw(cx, file_name, directory)
 }
 
 pub fn unknown_file_metadata(cx: &CodegenCx<'ll, '_>) -> &'ll DIFile {
-    file_metadata_raw(cx, "<unknown>", None, "", None)
+    file_metadata_raw(cx, None, None)
 }
 
 fn file_metadata_raw(cx: &CodegenCx<'ll, '_>,
-                     file_name: &str,
-                     file_name_symbol: Option<Symbol>,
-                     directory: &str,
-                     directory_symbol: Option<Symbol>)
+                     file_name: Option<String>,
+                     directory: Option<String>)
                      -> &'ll DIFile {
-    let key = (file_name_symbol, directory_symbol);
+    let key = (file_name, directory);
+
+    match debug_context(cx).created_files.borrow_mut().entry(key) {
+        Entry::Occupied(o) => return o.get(),
+        Entry::Vacant(v) => {
+            let (file_name, directory) = v.key();
+            debug!("file_metadata: file_name: {:?}, directory: {:?}", file_name, directory);
+
+            let file_name = SmallCStr::new(
+                if let Some(file_name) = file_name { &file_name } else { "<unknown>" });
+            let directory = SmallCStr::new(
+                if let Some(directory) = directory { &directory } else { "" });
+
+            let file_metadata = unsafe {
+                llvm::LLVMRustDIBuilderCreateFile(DIB(cx),
+                                                  file_name.as_ptr(),
+                                                  directory.as_ptr())
+            };
 
-    if let Some(file_metadata) = debug_context(cx).created_files.borrow().get(&key) {
-        return *file_metadata;
+            v.insert(file_metadata);
+            file_metadata
+        }
     }
-
-    debug!("file_metadata: file_name: {}, directory: {}", file_name, directory);
-
-    let file_name = SmallCStr::new(file_name);
-    let directory = SmallCStr::new(directory);
-
-    let file_metadata = unsafe {
-        llvm::LLVMRustDIBuilderCreateFile(DIB(cx),
-                                          file_name.as_ptr(),
-                                          directory.as_ptr())
-    };
-
-    let mut created_files = debug_context(cx).created_files.borrow_mut();
-    created_files.insert(key, file_metadata);
-    file_metadata
 }
 
 fn basic_type_metadata(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) -> &'ll DIType {
diff --git a/src/librustc_codegen_llvm/debuginfo/mod.rs b/src/librustc_codegen_llvm/debuginfo/mod.rs
index f3070a03b4ed5..527290392fff4 100644
--- a/src/librustc_codegen_llvm/debuginfo/mod.rs
+++ b/src/librustc_codegen_llvm/debuginfo/mod.rs
@@ -37,7 +37,7 @@ use std::ffi::CString;
 
 use syntax_pos::{self, Span, Pos};
 use syntax::ast;
-use syntax::symbol::{Symbol, InternedString};
+use syntax::symbol::InternedString;
 use rustc::ty::layout::{self, LayoutOf, HasTyCtxt};
 use rustc_codegen_ssa::traits::*;
 
@@ -63,7 +63,7 @@ pub struct CrateDebugContext<'a, 'tcx> {
     llcontext: &'a llvm::Context,
     llmod: &'a llvm::Module,
     builder: &'a mut DIBuilder<'a>,
-    created_files: RefCell<FxHashMap<(Option<Symbol>, Option<Symbol>), &'a DIFile>>,
+    created_files: RefCell<FxHashMap<(Option<String>, Option<String>), &'a DIFile>>,
     created_enum_disr_types: RefCell<FxHashMap<(DefId, layout::Primitive), &'a DIType>>,
 
     type_map: RefCell<TypeMap<'a, 'tcx>>,