Skip to content

Commit

Permalink
perf(runtime): optimize allocations in read/write checks (denoland#15631
Browse files Browse the repository at this point in the history
)
  • Loading branch information
littledivy committed Aug 27, 2022
1 parent 19fb9ab commit 8e1c0e5
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 17 deletions.
1 change: 1 addition & 0 deletions core/normalize_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
11 changes: 5 additions & 6 deletions runtime/fs_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
40 changes: 29 additions & 11 deletions runtime/permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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!(
Expand All @@ -79,19 +79,34 @@ 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 => {
Self::log_perm_access(name, info);
(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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 8e1c0e5

Please sign in to comment.