From 8e1c0e5141fa93f9169c28f32093bb7b30cd4e05 Mon Sep 17 00:00:00 2001
From: Divy Srivastava <dj.srivastava23@gmail.com>
Date: Sat, 27 Aug 2022 20:50:05 +0530
Subject: [PATCH] perf(runtime): optimize allocations in read/write checks
 (#15631)

---
 core/normalize_path.rs |  1 +
 runtime/fs_util.rs     | 11 +++++------
 runtime/permissions.rs | 40 +++++++++++++++++++++++++++++-----------
 3 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/core/normalize_path.rs b/core/normalize_path.rs
index 150f30b590..a295b55e64 100644
--- a/core/normalize_path.rs
+++ b/core/normalize_path.rs
@@ -9,6 +9,7 @@ use std::path::PathBuf;
 ///
 /// Taken from Cargo
 /// <https://github.com/rust-lang/cargo/blob/af307a38c20a753ec60f0ad18be5abed3db3c9ac/src/cargo/util/paths.rs#L60-L85>
+#[inline]
 pub fn normalize_path<P: AsRef<Path>>(path: P) -> PathBuf {
   let mut components = path.as_ref().components().peekable();
   let mut ret =
diff --git a/runtime/fs_util.rs b/runtime/fs_util.rs
index 02bcddd76d..30598e0419 100644
--- a/runtime/fs_util.rs
+++ b/runtime/fs_util.rs
@@ -21,16 +21,15 @@ pub fn canonicalize_path(path: &Path) -> Result<PathBuf, Error> {
   Ok(canonicalized_path)
 }
 
+#[inline]
 pub fn resolve_from_cwd(path: &Path) -> Result<PathBuf, AnyError> {
-  let resolved_path = if path.is_absolute() {
-    path.to_owned()
+  if path.is_absolute() {
+    Ok(normalize_path(path))
   } else {
     let cwd =
       current_dir().context("Failed to get current working directory")?;
-    cwd.join(path)
-  };
-
-  Ok(normalize_path(&resolved_path))
+    Ok(normalize_path(&cwd.join(path)))
+  }
 }
 
 #[cfg(test)]
diff --git a/runtime/permissions.rs b/runtime/permissions.rs
index 5893f44d46..01143e7a67 100644
--- a/runtime/permissions.rs
+++ b/runtime/permissions.rs
@@ -43,7 +43,7 @@ pub enum PermissionState {
 
 impl PermissionState {
   #[inline(always)]
-  fn log_perm_access(name: &str, info: Option<&str>) {
+  fn log_perm_access(name: &str, info: impl FnOnce() -> Option<String>) {
     // Eliminates log overhead (when logging is disabled),
     // log_enabled!(Debug) check in a hot path still has overhead
     // TODO(AaronO): generalize or upstream this optimization
@@ -59,15 +59,15 @@ impl PermissionState {
     }
   }
 
-  fn fmt_access(name: &str, info: Option<&str>) -> String {
+  fn fmt_access(name: &str, info: impl FnOnce() -> Option<String>) -> String {
     format!(
       "{} access{}",
       name,
-      info.map_or(String::new(), |info| { format!(" to {}", info) }),
+      info().map_or(String::new(), |info| { format!(" to {}", info) }),
     )
   }
 
-  fn error(name: &str, info: Option<&str>) -> AnyError {
+  fn error(name: &str, info: impl FnOnce() -> Option<String>) -> AnyError {
     custom_error(
       "PermissionDenied",
       format!(
@@ -79,11 +79,22 @@ impl PermissionState {
   }
 
   /// Check the permission state. bool is whether a prompt was issued.
+  #[inline]
   fn check(
     self,
     name: &str,
     info: Option<&str>,
     prompt: bool,
+  ) -> (Result<(), AnyError>, bool) {
+    self.check2(name, || info.map(|s| s.to_string()), prompt)
+  }
+
+  #[inline]
+  fn check2(
+    self,
+    name: &str,
+    info: impl Fn() -> Option<String>,
+    prompt: bool,
   ) -> (Result<(), AnyError>, bool) {
     match self {
       PermissionState::Granted => {
@@ -91,7 +102,11 @@ impl PermissionState {
         (Ok(()), false)
       }
       PermissionState::Prompt if prompt => {
-        let msg = Self::fmt_access(name, info);
+        let msg = format!(
+          "{} access{}",
+          name,
+          info().map_or(String::new(), |info| { format!(" to {}", info) }),
+        );
         if permission_prompt(&msg, name) {
           Self::log_perm_access(name, info);
           (Ok(()), true)
@@ -367,14 +382,15 @@ impl UnaryPermission<ReadDescriptor> {
     self.query(path)
   }
 
+  #[inline]
   pub fn check(&mut self, path: &Path) -> Result<(), AnyError> {
-    let (resolved_path, display_path) = resolved_and_display_path(path);
-    let (result, prompted) = self.query(Some(&resolved_path)).check(
+    let (result, prompted) = self.query(Some(path)).check2(
       self.name,
-      Some(&format!("\"{}\"", display_path.display())),
+      || Some(format!("\"{}\"", path.to_path_buf().display())),
       self.prompt,
     );
     if prompted {
+      let resolved_path = resolve_from_cwd(path).unwrap();
       if result.is_ok() {
         self.granted_list.insert(ReadDescriptor(resolved_path));
       } else {
@@ -518,14 +534,15 @@ impl UnaryPermission<WriteDescriptor> {
     self.query(path)
   }
 
+  #[inline]
   pub fn check(&mut self, path: &Path) -> Result<(), AnyError> {
-    let (resolved_path, display_path) = resolved_and_display_path(path);
-    let (result, prompted) = self.query(Some(&resolved_path)).check(
+    let (result, prompted) = self.query(Some(path)).check2(
       self.name,
-      Some(&format!("\"{}\"", display_path.display())),
+      || Some(format!("\"{}\"", path.to_path_buf().display())),
       self.prompt,
     );
     if prompted {
+      let resolved_path = resolve_from_cwd(path).unwrap();
       if result.is_ok() {
         self.granted_list.insert(WriteDescriptor(resolved_path));
       } else {
@@ -1459,6 +1476,7 @@ pub fn resolve_ffi_allowlist(
 
 /// Arbitrary helper. Resolves the path from CWD, and also gets a path that
 /// can be displayed without leaking the CWD when not allowed.
+#[inline]
 fn resolved_and_display_path(path: &Path) -> (PathBuf, PathBuf) {
   let resolved_path = resolve_from_cwd(path).unwrap();
   let display_path = path.to_path_buf();