Skip to content

Commit

Permalink
perf(node/fs): faster existsSync when not exists (denoland#21458)
Browse files Browse the repository at this point in the history
  • Loading branch information
dsherret committed Dec 4, 2023
1 parent 9eb25e3 commit a24d3e8
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 13 deletions.
39 changes: 38 additions & 1 deletion cli/tests/unit_node/fs_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
import { assert, assertThrows } from "../../../test_util/std/assert/mod.ts";
import { join } from "node:path";
import { tmpdir } from "node:os";
import { mkdtempSync, readFileSync, writeFileSync } from "node:fs";
import { existsSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs";
import { pathToAbsoluteFileUrl } from "../unit/test_util.ts";

Deno.test(
"[node/fs writeFileSync] write file without option",
Expand Down Expand Up @@ -43,3 +44,39 @@ Deno.test(
);
},
);

Deno.test(
"[node/fs existsSync] path",
{ permissions: { read: true } },
() => {
assert(existsSync("cli/tests/testdata/assets/fixture.json"));
},
);

Deno.test(
"[node/fs existsSync] url",
{ permissions: { read: true } },
() => {
assert(existsSync(
pathToAbsoluteFileUrl("cli/tests/testdata/assets/fixture.json"),
));
},
);

Deno.test(
"[node/fs existsSync] no permission",
{ permissions: { read: false } },
() => {
assertThrows(() => {
existsSync("cli/tests/testdata/assets/fixture.json");
}, Deno.errors.PermissionDenied);
},
);

Deno.test(
"[node/fs existsSync] not exists",
{ permissions: { read: true } },
() => {
assert(!existsSync("bad_filename"));
},
);
17 changes: 15 additions & 2 deletions ext/node/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,15 @@ pub trait NodePermissions {
url: &Url,
api_name: &str,
) -> Result<(), AnyError>;
fn check_read(&self, path: &Path) -> Result<(), AnyError>;
#[inline(always)]
fn check_read(&self, path: &Path) -> Result<(), AnyError> {
self.check_read_with_api_name(path, None)
}
fn check_read_with_api_name(
&self,
path: &Path,
api_name: Option<&str>,
) -> Result<(), AnyError>;
fn check_sys(&self, kind: &str, api_name: &str) -> Result<(), AnyError>;
}

Expand All @@ -64,7 +72,11 @@ impl NodePermissions for AllowAllNodePermissions {
) -> Result<(), AnyError> {
Ok(())
}
fn check_read(&self, _path: &Path) -> Result<(), AnyError> {
fn check_read_with_api_name(
&self,
_path: &Path,
_api_name: Option<&str>,
) -> Result<(), AnyError> {
Ok(())
}
fn check_sys(&self, _kind: &str, _api_name: &str) -> Result<(), AnyError> {
Expand Down Expand Up @@ -227,6 +239,7 @@ deno_core::extension!(deno_node,
ops::crypto::x509::op_node_x509_get_valid_to,
ops::crypto::x509::op_node_x509_get_serial_number,
ops::crypto::x509::op_node_x509_key_usage,
ops::fs::op_node_fs_exists_sync<P>,
ops::winerror::op_node_sys_to_uv_error,
ops::v8::op_v8_cached_data_version_tag,
ops::v8::op_v8_get_heap_statistics,
Expand Down
26 changes: 26 additions & 0 deletions ext/node/ops/fs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.

use std::path::PathBuf;

use deno_core::error::AnyError;
use deno_core::op2;
use deno_core::OpState;
use deno_fs::FileSystemRc;

use crate::NodePermissions;

#[op2(fast)]
pub fn op_node_fs_exists_sync<P>(
state: &mut OpState,
#[string] path: String,
) -> Result<bool, AnyError>
where
P: NodePermissions + 'static,
{
let path = PathBuf::from(path);
state
.borrow_mut::<P>()
.check_read_with_api_name(&path, Some("node:fs.existsSync()"))?;
let fs = state.borrow::<FileSystemRc>();
Ok(fs.lstat_sync(&path).is_ok())
}
1 change: 1 addition & 0 deletions ext/node/ops/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.

pub mod crypto;
pub mod fs;
pub mod http;
pub mod http2;
pub mod idna;
Expand Down
9 changes: 2 additions & 7 deletions ext/node/polyfills/_fs/_fs_exists.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

// TODO(petamoriken): enable prefer-primordials for node polyfills
// deno-lint-ignore-file prefer-primordials

const core = globalThis.__bootstrap.core;
import { pathFromURL } from "ext:deno_web/00_infra.js";

type ExistsCallback = (exists: boolean) => void;
Expand Down Expand Up @@ -35,10 +35,5 @@ Object.defineProperty(exists, kCustomPromisifiedSymbol, {
*/
export function existsSync(path: string | URL): boolean {
path = path instanceof URL ? pathFromURL(path) : path;
try {
Deno.lstatSync(path);
return true;
} catch (_err) {
return false;
}
return core.ops.op_node_fs_exists_sync(path);
}
8 changes: 6 additions & 2 deletions runtime/permissions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1376,8 +1376,12 @@ impl deno_node::NodePermissions for PermissionsContainer {
}

#[inline(always)]
fn check_read(&self, path: &Path) -> Result<(), AnyError> {
self.0.lock().read.check(path, None)
fn check_read_with_api_name(
&self,
path: &Path,
api_name: Option<&str>,
) -> Result<(), AnyError> {
self.0.lock().read.check(path, api_name)
}

fn check_sys(&self, kind: &str, api_name: &str) -> Result<(), AnyError> {
Expand Down
6 changes: 5 additions & 1 deletion runtime/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,11 @@ impl deno_node::NodePermissions for Permissions {
) -> Result<(), deno_core::error::AnyError> {
unreachable!("snapshotting!")
}
fn check_read(&self, _p: &Path) -> Result<(), deno_core::error::AnyError> {
fn check_read_with_api_name(
&self,
_p: &Path,
_api_name: Option<&str>,
) -> Result<(), deno_core::error::AnyError> {
unreachable!("snapshotting!")
}
fn check_sys(
Expand Down

0 comments on commit a24d3e8

Please sign in to comment.