perf(core): Cache source lookups (#14816)

Keep a cache for source maps and source lines. 

We sort of already had a cache argument for source map lookup 
functions but we just passed an empty map instead of storing it. 

Extended it to cache source line lookups as well and plugged it 
into runtime state.

6 files changed, 147 insertions, 152 deletions

diff --git a/cli/fmt_errors.rs b/cli/fmt_errors.rs
index d30f661a9..37a58364a 100644
--- a/cli/fmt_errors.rs
+++ b/cli/fmt_errors.rs
@@ -8,8 +8,6 @@ use deno_core::error::format_file_name;
 use deno_core::error::JsError;
 use deno_core::error::JsStackFrame;
 
-const SOURCE_ABBREV_THRESHOLD: usize = 150;
-
 // Keep in sync with `/core/error.js`.
 pub fn format_location(frame: &JsStackFrame) -> String {
   let _internal = frame
@@ -115,8 +113,7 @@ fn format_maybe_source_line(
   let source_line = source_line.unwrap();
   // sometimes source_line gets set with an empty string, which then outputs
   // an empty source line when displayed, so need just short circuit here.
-  // Also short-circuit on error line too long.
-  if source_line.is_empty() || source_line.len() > SOURCE_ABBREV_THRESHOLD {
+  if source_line.is_empty() {
     return "".to_string();
   }
   if source_line.contains("Couldn't format source line: ") {
diff --git a/cli/proc_state.rs b/cli/proc_state.rs
index 2c454c0ee..9e948643d 100644
--- a/cli/proc_state.rs
+++ b/cli/proc_state.rs
@@ -694,11 +694,9 @@ impl SourceMapGetter for ProcState {
         _ => return None,
       }
       if let Some((code, maybe_map)) = self.get_emit(&specifier) {
-        let code = String::from_utf8(code).unwrap();
-        source_map_from_code(code).or(maybe_map)
+        source_map_from_code(&code).or(maybe_map)
       } else if let Ok(source) = self.load(specifier, None, false) {
-        let code = String::from_utf8(source.code.to_vec()).unwrap();
-        source_map_from_code(code)
+        source_map_from_code(&source.code)
       } else {
         None
       }
@@ -756,21 +754,14 @@ pub fn import_map_from_text(
   Ok(result.import_map)
 }
 
-fn source_map_from_code(code: String) -> Option<Vec<u8>> {
-  let lines: Vec<&str> = code.split('\n').collect();
-  if let Some(last_line) = lines.last() {
-    if last_line
-      .starts_with("//# sourceMappingURL=data:application/json;base64,")
-    {
-      let input = last_line.trim_start_matches(
-        "//# sourceMappingURL=data:application/json;base64,",
-      );
-      let decoded_map = base64::decode(input)
-        .expect("Unable to decode source map from emitted file.");
-      Some(decoded_map)
-    } else {
-      None
-    }
+fn source_map_from_code(code: &[u8]) -> Option<Vec<u8>> {
+  static PREFIX: &[u8] = b"//# sourceMappingURL=data:application/json;base64,";
+  let last_line = code.rsplitn(2, |u| u == &b'\n').next().unwrap();
+  if last_line.starts_with(PREFIX) {
+    let input = last_line.split_at(PREFIX.len()).1;
+    let decoded_map = base64::decode(input)
+      .expect("Unable to decode source map from emitted file.");
+    Some(decoded_map)
   } else {
     None
   }
diff --git a/core/error.rs b/core/error.rs
index c3bf6e861..f193c13e2 100644
--- a/core/error.rs
+++ b/core/error.rs
@@ -2,10 +2,10 @@
 
 use crate::runtime::JsRuntime;
 use crate::source_map::apply_source_map;
+use crate::source_map::get_source_line;
 use crate::url::Url;
 use anyhow::Error;
 use std::borrow::Cow;
-use std::collections::HashMap;
 use std::collections::HashSet;
 use std::fmt;
 use std::fmt::Debug;
@@ -192,15 +192,20 @@ impl JsError {
     let msg = v8::Exception::create_message(scope, exception);
 
     let mut exception_message = None;
-    let state_rc = JsRuntime::state(scope);
-    let state = state_rc.borrow();
-    if let Some(format_exception_cb) = &state.js_format_exception_cb {
-      let format_exception_cb = format_exception_cb.open(scope);
-      let this = v8::undefined(scope).into();
-      let formatted = format_exception_cb.call(scope, this, &[exception]);
-      if let Some(formatted) = formatted {
-        if formatted.is_string() {
-          exception_message = Some(formatted.to_rust_string_lossy(scope));
+    // Nest this state borrow. A mutable borrow can occur when accessing `stack`
+    // in this outer scope, invoking `Error.prepareStackTrace()` which calls
+    // `op_apply_source_map`.
+    {
+      let state_rc = JsRuntime::state(scope);
+      let state = state_rc.borrow();
+      if let Some(format_exception_cb) = &state.js_format_exception_cb {
+        let format_exception_cb = format_exception_cb.open(scope);
+        let this = v8::undefined(scope).into();
+        let formatted = format_exception_cb.call(scope, this, &[exception]);
+        if let Some(formatted) = formatted {
+          if formatted.is_string() {
+            exception_message = Some(formatted.to_rust_string_lossy(scope));
+          }
         }
       }
     }
@@ -255,66 +260,73 @@ impl JsError {
         None => vec![],
       };
 
-      let state_rc = JsRuntime::state(scope);
-      let state = state_rc.borrow();
-
-      // When the stack frame array is empty, but the source location given by
-      // (script_resource_name, line_number, start_column + 1) exists, this is
-      // likely a syntax error. For the sake of formatting we treat it like it
-      // was given as a single stack frame.
-      if frames.is_empty() {
-        let script_resource_name = msg
-          .get_script_resource_name(scope)
-          .and_then(|v| v8::Local::<v8::String>::try_from(v).ok())
-          .map(|v| v.to_rust_string_lossy(scope));
-        let line_number: Option<i64> =
-          msg.get_line_number(scope).and_then(|v| v.try_into().ok());
-        let column_number: Option<i64> = msg.get_start_column().try_into().ok();
-        if let (Some(f), Some(l), Some(c)) =
-          (script_resource_name, line_number, column_number)
-        {
-          // V8's column numbers are 0-based, we want 1-based.
-          let c = c + 1;
-          if let Some(source_map_getter) = &state.source_map_getter {
-            let (f, l, c, _) = apply_source_map(
-              f,
-              l,
-              c,
-              &mut HashMap::new(),
-              source_map_getter.as_ref(),
-            );
-            frames =
-              vec![JsStackFrame::from_location(Some(f), Some(l), Some(c))];
-          } else {
-            frames =
-              vec![JsStackFrame::from_location(Some(f), Some(l), Some(c))];
-          }
-        }
-      }
-
       let mut source_line = None;
       let mut source_line_frame_index = None;
-      if let Some(source_map_getter) = &state.source_map_getter {
-        for (i, frame) in frames.iter().enumerate() {
-          if let (Some(file_name), Some(line_number)) =
-            (&frame.file_name, frame.line_number)
+      {
+        let state_rc = JsRuntime::state(scope);
+        let state = &mut *state_rc.borrow_mut();
+
+        // When the stack frame array is empty, but the source location given by
+        // (script_resource_name, line_number, start_column + 1) exists, this is
+        // likely a syntax error. For the sake of formatting we treat it like it
+        // was given as a single stack frame.
+        if frames.is_empty() {
+          let script_resource_name = msg
+            .get_script_resource_name(scope)
+            .and_then(|v| v8::Local::<v8::String>::try_from(v).ok())
+            .map(|v| v.to_rust_string_lossy(scope));
+          let line_number: Option<i64> =
+            msg.get_line_number(scope).and_then(|v| v.try_into().ok());
+          let column_number: Option<i64> =
+            msg.get_start_column().try_into().ok();
+          if let (Some(f), Some(l), Some(c)) =
+            (script_resource_name, line_number, column_number)
           {
-            if !file_name.trim_start_matches('[').starts_with("deno:") {
-              // Source lookup expects a 0-based line number, ours are 1-based.
-              source_line = source_map_getter
-                .get_source_line(file_name, (line_number - 1) as usize);
-              source_line_frame_index = Some(i);
-              break;
+            // V8's column numbers are 0-based, we want 1-based.
+            let c = c + 1;
+            if let Some(source_map_getter) = &state.source_map_getter {
+              let (f, l, c) = apply_source_map(
+                f,
+                l,
+                c,
+                &mut state.source_map_cache,
+                source_map_getter.as_ref(),
+              );
+              frames =
+                vec![JsStackFrame::from_location(Some(f), Some(l), Some(c))];
+            } else {
+              frames =
+                vec![JsStackFrame::from_location(Some(f), Some(l), Some(c))];
             }
           }
         }
-      } else if let Some(frame) = frames.first() {
-        if let Some(file_name) = &frame.file_name {
-          if !file_name.trim_start_matches('[').starts_with("deno:") {
-            source_line = msg
-              .get_source_line(scope)
-              .map(|v| v.to_rust_string_lossy(scope));
-            source_line_frame_index = Some(0);
+
+        if let Some(source_map_getter) = &state.source_map_getter {
+          for (i, frame) in frames.iter().enumerate() {
+            if let (Some(file_name), Some(line_number)) =
+              (&frame.file_name, frame.line_number)
+            {
+              if !file_name.trim_start_matches('[').starts_with("deno:") {
+                // Source lookup expects a 0-based line number, ours are 1-based.
+                source_line = get_source_line(
+                  file_name,
+                  line_number,
+                  &mut state.source_map_cache,
+                  source_map_getter.as_ref(),
+                );
+                source_line_frame_index = Some(i);
+                break;
+              }
+            }
+          }
+        } else if let Some(frame) = frames.first() {
+          if let Some(file_name) = &frame.file_name {
+            if !file_name.trim_start_matches('[').starts_with("deno:") {
+              source_line = msg
+                .get_source_line(scope)
+                .map(|v| v.to_rust_string_lossy(scope));
+              source_line_frame_index = Some(0);
+            }
           }
         }
       }
diff --git a/core/ops_builtin_v8.rs b/core/ops_builtin_v8.rs
index 7b7cfcc45..7f0c58212 100644
--- a/core/ops_builtin_v8.rs
+++ b/core/ops_builtin_v8.rs
@@ -752,14 +752,14 @@ fn op_apply_source_map(
   location: Location,
 ) -> Result<Location, Error> {
   let state_rc = JsRuntime::state(scope);
-  let state = state_rc.borrow();
+  let state = &mut *state_rc.borrow_mut();
   if let Some(source_map_getter) = &state.source_map_getter {
     let mut location = location;
-    let (f, l, c, _) = apply_source_map_(
+    let (f, l, c) = apply_source_map_(
       location.file_name,
       location.line_number.into(),
       location.column_number.into(),
-      &mut Default::default(),
+      &mut state.source_map_cache,
       source_map_getter.as_ref(),
     );
     location.file_name = f;
diff --git a/core/runtime.rs b/core/runtime.rs
index 4ca1247f6..97c822848 100644
--- a/core/runtime.rs
+++ b/core/runtime.rs
@@ -17,6 +17,7 @@ use crate::modules::NoopModuleLoader;
 use crate::op_void_async;
 use crate::op_void_sync;
 use crate::ops::*;
+use crate::source_map::SourceMapCache;
 use crate::source_map::SourceMapGetter;
 use crate::Extension;
 use crate::OpMiddlewareFn;
@@ -163,6 +164,7 @@ pub(crate) struct JsRuntimeState {
   /// of the event loop.
   dyn_module_evaluate_idle_counter: u32,
   pub(crate) source_map_getter: Option<Box<dyn SourceMapGetter>>,
+  pub(crate) source_map_cache: SourceMapCache,
   pub(crate) pending_ops: FuturesUnordered<PendingOpFuture>,
   pub(crate) unrefed_ops: HashSet<i32>,
   pub(crate) have_unpolled_ops: bool,
@@ -391,6 +393,7 @@ impl JsRuntime {
       has_tick_scheduled: false,
       js_wasm_streaming_cb: None,
       source_map_getter: options.source_map_getter,
+      source_map_cache: Default::default(),
       pending_ops: FuturesUnordered::new(),
       unrefed_ops: HashSet::new(),
       shared_array_buffer_store: options.shared_array_buffer_store,
diff --git a/core/source_map.rs b/core/source_map.rs
index 770b29cc7..6a261fa7d 100644
--- a/core/source_map.rs
+++ b/core/source_map.rs
@@ -17,83 +17,75 @@ pub trait SourceMapGetter {
   ) -> Option<String>;
 }
 
-/// Cached filename lookups. The key can be None if a previous lookup failed to
-/// find a SourceMap.
-pub type CachedMaps = HashMap<String, Option<SourceMap>>;
+#[derive(Debug, Default)]
+pub struct SourceMapCache {
+  maps: HashMap<String, Option<SourceMap>>,
+  source_lines: HashMap<(String, i64), Option<String>>,
+}
 
 pub fn apply_source_map<G: SourceMapGetter + ?Sized>(
   file_name: String,
   line_number: i64,
   column_number: i64,
-  mappings_map: &mut CachedMaps,
+  cache: &mut SourceMapCache,
   getter: &G,
-) -> (String, i64, i64, Option<String>) {
+) -> (String, i64, i64) {
   // Lookup expects 0-based line and column numbers, but ours are 1-based.
   let line_number = line_number - 1;
   let column_number = column_number - 1;
 
-  let default_pos = (file_name.clone(), line_number, column_number, None);
-  let maybe_source_map = get_mappings(&file_name, mappings_map, getter);
-  let (file_name, line_number, column_number, source_line) =
-    match maybe_source_map {
-      None => default_pos,
-      Some(source_map) => {
-        match source_map.lookup_token(line_number as u32, column_number as u32)
-        {
+  let default_pos = (file_name.clone(), line_number, column_number);
+  let maybe_source_map =
+    cache.maps.entry(file_name.clone()).or_insert_with(|| {
+      getter
+        .get_source_map(&file_name)
+        .and_then(|raw_source_map| SourceMap::from_slice(&raw_source_map).ok())
+    });
+  let (file_name, line_number, column_number) = match maybe_source_map {
+    None => default_pos,
+    Some(source_map) => {
+      match source_map.lookup_token(line_number as u32, column_number as u32) {
+        None => default_pos,
+        Some(token) => match token.get_source() {
           None => default_pos,
-          Some(token) => match token.get_source() {
-            None => default_pos,
-            Some(source_file_name) => {
-              // The `source_file_name` written by tsc in the source map is
-              // sometimes only the basename of the URL, or has unwanted `<`/`>`
-              // around it. Use the `file_name` we get from V8 if
-              // `source_file_name` does not parse as a URL.
-              let file_name = match resolve_url(source_file_name) {
-                Ok(m) if m.scheme() == "blob" => file_name,
-                Ok(m) => m.to_string(),
-                Err(_) => file_name,
-              };
-              let source_line =
-                if let Some(source_view) = token.get_source_view() {
-                  source_view
-                    .get_line(token.get_src_line())
-                    .map(|s| s.to_string())
-                } else {
-                  None
-                };
-              (
-                file_name,
-                i64::from(token.get_src_line()),
-                i64::from(token.get_src_col()),
-                source_line,
-              )
-            }
-          },
-        }
+          Some(source_file_name) => {
+            // The `source_file_name` written by tsc in the source map is
+            // sometimes only the basename of the URL, or has unwanted `<`/`>`
+            // around it. Use the `file_name` we get from V8 if
+            // `source_file_name` does not parse as a URL.
+            let file_name = match resolve_url(source_file_name) {
+              Ok(m) if m.scheme() == "blob" => file_name,
+              Ok(m) => m.to_string(),
+              Err(_) => file_name,
+            };
+            (
+              file_name,
+              i64::from(token.get_src_line()),
+              i64::from(token.get_src_col()),
+            )
+          }
+        },
       }
-    };
-  let source_line = source_line
-    .or_else(|| getter.get_source_line(&file_name, line_number as usize));
-  (file_name, line_number + 1, column_number + 1, source_line)
+    }
+  };
+  (file_name, line_number + 1, column_number + 1)
 }
 
-fn get_mappings<'a, G: SourceMapGetter + ?Sized>(
-  file_name: &str,
-  mappings_map: &'a mut CachedMaps,
-  getter: &G,
-) -> &'a Option<SourceMap> {
-  mappings_map
-    .entry(file_name.to_string())
-    .or_insert_with(|| parse_map_string(file_name, getter))
-}
+const MAX_SOURCE_LINE_LENGTH: usize = 150;
 
-// TODO(kitsonk) parsed source maps should probably be cached in state in
-// the module meta data.
-fn parse_map_string<G: SourceMapGetter + ?Sized>(
+pub fn get_source_line<G: SourceMapGetter + ?Sized>(
   file_name: &str,
+  line_number: i64,
+  cache: &mut SourceMapCache,
   getter: &G,
-) -> Option<SourceMap> {
-  getter
-    .get_source_map(file_name)
-    .and_then(|raw_source_map| SourceMap::from_slice(&raw_source_map).ok())
+) -> Option<String> {
+  cache
+    .source_lines
+    .entry((file_name.to_string(), line_number))
+    .or_insert_with(|| {
+      // Source lookup expects a 0-based line number, ours are 1-based.
+      let s = getter.get_source_line(file_name, (line_number - 1) as usize);
+      s.filter(|s| s.len() <= MAX_SOURCE_LINE_LENGTH)
+    })
+    .clone()
 }