From 5326a998c7492c09f4fa710187428ba83c981fe8 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Sat, 14 Dec 2024 04:48:57 -0800 Subject: [PATCH] Don't open node_modules 1,618 times (#15762) --- src/cli/pm_trusted_command.zig | 8 +- src/install/install.zig | 153 ++++++++++++++++++++++++--------- src/install/lockfile.zig | 8 +- 3 files changed, 120 insertions(+), 49 deletions(-) diff --git a/src/cli/pm_trusted_command.zig b/src/cli/pm_trusted_command.zig index a26f46a4f08b1b..1341a9aa4703fe 100644 --- a/src/cli/pm_trusted_command.zig +++ b/src/cli/pm_trusted_command.zig @@ -97,11 +97,11 @@ pub const UntrustedCommand = struct { const package_id = pm.lockfile.buffers.resolutions.items[dep_id]; const resolution = &resolutions[package_id]; var package_scripts = scripts[package_id]; - + var not_lazy: PackageManager.LazyPackageDestinationDir = .{ .dir = node_modules_dir }; const maybe_scripts_list = package_scripts.getList( pm.log, pm.lockfile, - node_modules_dir, + ¬_lazy, abs_node_modules_path.items, alias, resolution, @@ -262,11 +262,11 @@ pub const TrustCommand = struct { } const resolution = &resolutions[package_id]; var package_scripts = scripts[package_id]; - + var not_lazy = PackageManager.LazyPackageDestinationDir{ .dir = node_modules_dir }; const maybe_scripts_list = package_scripts.getList( pm.log, pm.lockfile, - node_modules_dir, + ¬_lazy, abs_node_modules_path.items, alias, resolution, diff --git a/src/install/install.zig b/src/install/install.zig index bcc3d8cb506c30..59d8ec654128ce 100644 --- a/src/install/install.zig +++ b/src/install/install.zig @@ -1210,12 +1210,7 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type { const package_json_path: [:0]u8 = this.destination_dir_subpath_buf[0 .. this.destination_dir_subpath.len + std.fs.path.sep_str.len + "package.json".len :0]; defer this.destination_dir_subpath_buf[this.destination_dir_subpath.len] = 0; - var destination_dir = this.node_modules.openDir(root_node_modules_dir) catch return null; - defer { - if (std.fs.cwd().fd != destination_dir.fd) destination_dir.close(); - } - - var package_json_file = File.openat(destination_dir, package_json_path, bun.O.RDONLY, 0).unwrap() catch return null; + var package_json_file = this.node_modules.openPackageJSON(root_node_modules_dir, package_json_path) catch return null; defer package_json_file.close(); // Heuristic: most package.jsons will be less than 2048 bytes. @@ -1339,6 +1334,9 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type { .valid = true, .update_lockfile_pointers = true, }; + } else if (bin.isUnset()) { + // It's not unset. There's no bin. + bin.unset = 0; } return .{ @@ -12081,6 +12079,40 @@ pub const PackageManager = struct { } } + pub const LazyPackageDestinationDir = union(enum) { + dir: std.fs.Dir, + node_modules_path: struct { + node_modules: *NodeModulesFolder, + root_node_modules_dir: std.fs.Dir, + }, + closed: void, + + pub fn getDir(this: *LazyPackageDestinationDir) !std.fs.Dir { + return switch (this.*) { + .dir => |dir| dir, + .node_modules_path => |lazy| brk: { + const dir = try lazy.node_modules.openDir(lazy.root_node_modules_dir); + this.* = .{ .dir = dir }; + break :brk dir; + }, + .closed => @panic("LazyPackageDestinationDir is closed! This should never happen. Why did this happen?! It's not your fault. Its our fault. We're sorry."), + }; + } + + pub fn close(this: *LazyPackageDestinationDir) void { + switch (this.*) { + .dir => { + if (this.dir.fd != std.fs.cwd().fd) { + this.dir.close(); + } + }, + .node_modules_path, .closed => {}, + } + + this.* = .{ .closed = {} }; + } + }; + pub const NodeModulesFolder = struct { tree_id: Lockfile.Tree.Id = 0, path: std.ArrayList(u8) = std.ArrayList(u8).init(bun.default_allocator), @@ -12089,9 +12121,40 @@ pub const PackageManager = struct { this.path.clearAndFree(); } + noinline fn openPackageJSONFileWithoutOpeningDirectories(this: *const NodeModulesFolder, root_node_modules_dir: std.fs.Dir, package_json_path: [:0]const u8) bun.sys.Maybe(bun.sys.File) { + var path_buf: bun.PathBuffer = undefined; + const parts: [2][]const u8 = .{ this.path.items, package_json_path }; + return bun.sys.File.openat(bun.toFD(root_node_modules_dir), bun.path.joinZBuf(&path_buf, &parts, .auto), bun.O.RDONLY, 0); + } + + pub fn openPackageJSON(this: *const NodeModulesFolder, root_node_modules_dir: std.fs.Dir, package_json_path: [:0]const u8) !bun.sys.File { + if (this.path.items.len + package_json_path.len * 2 < bun.MAX_PATH_BYTES) { + // If we do not run the risk of ENAMETOOLONG, then let's just avoid opening the extra directories altogether. + switch (this.openPackageJSONFileWithoutOpeningDirectories(root_node_modules_dir, package_json_path)) { + .err => |e| { + switch (e.getErrno()) { + // Just incase we're wrong, let's try the fallback + .PERM, .ACCES, .INVAL, .NAMETOOLONG => { + // Use fallback + }, + else => return e.toZigErr(), + } + }, + .result => |file| return file, + } + } + + const dir = try this.openDir(root_node_modules_dir); + defer { + _ = bun.sys.close(bun.toFD(dir)); + } + + return try bun.sys.File.openat(bun.toFD(dir), package_json_path, bun.O.RDONLY, 0).unwrap(); + } + pub fn openDir(this: *const NodeModulesFolder, root: std.fs.Dir) !std.fs.Dir { if (comptime Environment.isPosix) { - return root.openDir(this.path.items, .{ .iterate = true, .access_sub_paths = true }); + return (try bun.sys.openat(bun.toFD(root), &try std.posix.toPosixPath(this.path.items), bun.O.DIRECTORY, 0).unwrap()).asDir(); } return (try bun.sys.openDirAtWindowsA(bun.toFD(root), this.path.items, .{ @@ -12194,7 +12257,7 @@ pub const PackageManager = struct { pub fn incrementTreeInstallCount( this: *PackageInstaller, tree_id: Lockfile.Tree.Id, - maybe_destination_dir: ?std.fs.Dir, + maybe_destination_dir: ?*LazyPackageDestinationDir, comptime should_install_packages: bool, comptime log_level: Options.LogLevel, ) void { @@ -12221,21 +12284,23 @@ pub const PackageManager = struct { this.completed_trees.set(tree_id); - if (maybe_destination_dir orelse (this.node_modules.makeAndOpenDir(this.root_node_modules_folder) catch null)) |_destination_dir| { - var destination_dir = _destination_dir; - defer { - if (maybe_destination_dir == null) { - destination_dir.close(); + if (maybe_destination_dir) |maybe| { + if (maybe.getDir() catch null) |_destination_dir| { + var destination_dir = _destination_dir; + defer { + if (maybe_destination_dir == null) { + destination_dir.close(); + } } - } - this.seen_bin_links.clearRetainingCapacity(); + this.seen_bin_links.clearRetainingCapacity(); - if (tree.binaries.count() > 0) { - var link_target_buf: bun.PathBuffer = undefined; - var link_dest_buf: bun.PathBuffer = undefined; - var link_rel_buf: bun.PathBuffer = undefined; - this.linkTreeBins(tree, tree_id, destination_dir, &link_target_buf, &link_dest_buf, &link_rel_buf, log_level); + if (tree.binaries.count() > 0) { + var link_target_buf: bun.PathBuffer = undefined; + var link_dest_buf: bun.PathBuffer = undefined; + var link_rel_buf: bun.PathBuffer = undefined; + this.linkTreeBins(tree, tree_id, destination_dir, &link_target_buf, &link_dest_buf, &link_rel_buf, log_level); + } } } @@ -12635,7 +12700,7 @@ pub const PackageManager = struct { alias: string, package_id: PackageID, resolution_tag: Resolution.Tag, - node_modules_folder: std.fs.Dir, + node_modules_folder: *LazyPackageDestinationDir, comptime log_level: Options.LogLevel, ) usize { if (comptime Environment.allow_assert) { @@ -13083,6 +13148,8 @@ pub const PackageManager = struct { if (std.fs.cwd().fd != destination_dir.fd) destination_dir.close(); } + var lazy_package_dir: LazyPackageDestinationDir = .{ .dir = destination_dir }; + const install_result = switch (resolution.tag) { .symlink, .workspace => installer.installFromLink(this.skip_delete, destination_dir), else => result: { @@ -13146,7 +13213,7 @@ pub const PackageManager = struct { if (this.enqueueLifecycleScripts( alias.slice(this.lockfile.buffers.string_bytes.items), log_level, - destination_dir, + &lazy_package_dir, package_id, dep.behavior.optional, resolution, @@ -13174,7 +13241,7 @@ pub const PackageManager = struct { alias.slice(this.lockfile.buffers.string_bytes.items), package_id, resolution.tag, - destination_dir, + &lazy_package_dir, log_level, ); if (count > 0) { @@ -13192,7 +13259,7 @@ pub const PackageManager = struct { }, } - if (!pkg_has_patch) this.incrementTreeInstallCount(this.current_tree_id, destination_dir, !is_pending_package_install, log_level); + if (!pkg_has_patch) this.incrementTreeInstallCount(this.current_tree_id, &lazy_package_dir, !is_pending_package_install, log_level); }, .fail => |cause| { if (comptime Environment.allow_assert) { @@ -13201,7 +13268,7 @@ pub const PackageManager = struct { // even if the package failed to install, we still need to increment the install // counter for this tree - if (!pkg_has_patch) this.incrementTreeInstallCount(this.current_tree_id, destination_dir, !is_pending_package_install, log_level); + if (!pkg_has_patch) this.incrementTreeInstallCount(this.current_tree_id, &lazy_package_dir, !is_pending_package_install, log_level); if (cause.err == error.DanglingSymlink) { Output.prettyErrorln( @@ -13220,7 +13287,15 @@ pub const PackageManager = struct { }; if (!Singleton.node_modules_is_ok) { if (!Environment.isWindows) { - const stat = bun.sys.fstat(bun.toFD(destination_dir)).unwrap() catch |err| { + const stat = bun.sys.fstat(bun.toFD(lazy_package_dir.getDir() catch |err| { + Output.err("EACCES", "Permission denied while installing {s}", .{ + this.names[package_id].slice(this.lockfile.buffers.string_bytes.items), + }); + if (Environment.isDebug) { + Output.err(err, "Failed to stat node_modules", .{}); + } + Global.exit(1); + })).unwrap() catch |err| { Output.err("EACCES", "Permission denied while installing {s}", .{ this.names[package_id].slice(this.lockfile.buffers.string_bytes.items), }); @@ -13274,23 +13349,18 @@ pub const PackageManager = struct { this.trees[this.current_tree_id].binaries.add(dependency_id) catch bun.outOfMemory(); } - var destination_dir = this.node_modules.makeAndOpenDir(this.root_node_modules_folder) catch |err| { - if (log_level != .silent) { - Output.err(err, "Failed to open node_modules folder for {s} in {s}", .{ - pkg_name.slice(this.lockfile.buffers.string_bytes.items), - bun.fmt.fmtPath(u8, this.node_modules.path.items, .{}), - }); - } - this.summary.fail += 1; - if (!pkg_has_patch) this.incrementTreeInstallCount(this.current_tree_id, null, !is_pending_package_install, log_level); - return; + var destination_dir: LazyPackageDestinationDir = .{ + .node_modules_path = .{ + .node_modules = &this.node_modules, + .root_node_modules_dir = this.root_node_modules_folder, + }, }; defer { - if (std.fs.cwd().fd != destination_dir.fd) destination_dir.close(); + destination_dir.close(); } - defer if (!pkg_has_patch) this.incrementTreeInstallCount(this.current_tree_id, destination_dir, !is_pending_package_install, log_level); + defer if (!pkg_has_patch) this.incrementTreeInstallCount(this.current_tree_id, &destination_dir, !is_pending_package_install, log_level); const dep = this.lockfile.buffers.dependencies.items[dependency_id]; const truncated_dep_name_hash: TruncatedPackageNameHash = @truncate(dep.name_hash); @@ -13309,7 +13379,7 @@ pub const PackageManager = struct { if (this.enqueueLifecycleScripts( alias.slice(this.lockfile.buffers.string_bytes.items), log_level, - destination_dir, + &destination_dir, package_id, dep.behavior.optional, resolution, @@ -13346,7 +13416,8 @@ pub const PackageManager = struct { this.manager.scopeForPackageName(pkg_name), pkg_name_hash, &expired, - .load_from_memory_fallback_to_disk, + // Do not fallback to disk. These files are much larger than the package.json + .load_from_memory, )) |manifest| { if (manifest.findByVersion(resolution.value.npm.version)) |find| { return find.package.bin.cloneAppend(manifest.string_buf, manifest.extern_strings_bin_entries, this.lockfile); @@ -13398,7 +13469,7 @@ pub const PackageManager = struct { this: *PackageInstaller, folder_name: string, comptime log_level: Options.LogLevel, - node_modules_folder: std.fs.Dir, + node_modules_folder: *LazyPackageDestinationDir, package_id: PackageID, optional: bool, resolution: *const Resolution, diff --git a/src/install/lockfile.zig b/src/install/lockfile.zig index 14493dcd7864b1..6563a01ab671c5 100644 --- a/src/install/lockfile.zig +++ b/src/install/lockfile.zig @@ -3367,7 +3367,7 @@ pub const Package = extern struct { this: *Package.Scripts, log: *logger.Log, lockfile: *Lockfile, - node_modules: std.fs.Dir, + node_modules: *PackageManager.LazyPackageDestinationDir, abs_node_modules_path: string, folder_name: string, resolution: *const Resolution, @@ -3427,13 +3427,13 @@ pub const Package = extern struct { allocator: std.mem.Allocator, string_builder: *Lockfile.StringBuilder, log: *logger.Log, - node_modules: std.fs.Dir, + node_modules: *PackageManager.LazyPackageDestinationDir, folder_name: string, ) !void { const json = brk: { const json_src = brk2: { const json_path = bun.path.joinZ([_]string{ folder_name, "package.json" }, .auto); - const buf = try bun.sys.File.readFrom(node_modules, json_path, allocator).unwrap(); + const buf = try bun.sys.File.readFrom(try node_modules.getDir(), json_path, allocator).unwrap(); break :brk2 logger.Source.initPathString(json_path, buf); }; @@ -3455,7 +3455,7 @@ pub const Package = extern struct { this: *Package.Scripts, log: *logger.Log, lockfile: *Lockfile, - node_modules: std.fs.Dir, + node_modules: *PackageManager.LazyPackageDestinationDir, abs_folder_path: string, folder_name: string, resolution_tag: Resolution.Tag,