From 260366f1a69e2d67e8a55a85b77ef6a264c85be5 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Thu, 23 May 2024 15:02:12 -0700 Subject: [PATCH] ~2x faster uncached `bun install` on Windows (#11293) --- src/install/install.zig | 10 ++++-- src/install/npm.zig | 78 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 76 insertions(+), 12 deletions(-) diff --git a/src/install/install.zig b/src/install/install.zig index 9527aa5bcd..7bb98c3390 100644 --- a/src/install/install.zig +++ b/src/install/install.zig @@ -2344,8 +2344,9 @@ pub const PackageManager = struct { cache_directory_: ?std.fs.Dir = null, // TODO(dylan-conway): remove this field when we move away from `std.ChildProcess` in repository.zig - cache_directory_path: string = "", + cache_directory_path: stringZ = "", temp_dir_: ?std.fs.Dir = null, + temp_dir_path: stringZ = "", temp_dir_name: string = "", root_dir: *Fs.FileSystem.DirEntry, allocator: std.mem.Allocator, @@ -3063,6 +3064,9 @@ pub const PackageManager = struct { pub inline fn getTemporaryDirectory(this: *PackageManager) std.fs.Dir { return this.temp_dir_ orelse brk: { this.temp_dir_ = this.ensureTemporaryDirectory(); + var pathbuf: bun.PathBuffer = undefined; + const temp_dir_path = bun.getFdPathZ(bun.toFD(this.temp_dir_.?), &pathbuf) catch Output.panic("Unable to read temporary directory path", .{}); + this.temp_dir_path = bun.default_allocator.dupeZ(u8, temp_dir_path) catch bun.outOfMemory(); break :brk this.temp_dir_.?; }; } @@ -3071,7 +3075,7 @@ pub const PackageManager = struct { loop: while (true) { if (this.options.enable.cache) { const cache_dir = fetchCacheDirectoryPath(this.env); - this.cache_directory_path = this.allocator.dupe(u8, cache_dir.path) catch bun.outOfMemory(); + this.cache_directory_path = this.allocator.dupeZ(u8, cache_dir.path) catch bun.outOfMemory(); return std.fs.cwd().makeOpenPath(cache_dir.path, .{}) catch { this.options.enable.cache = false; @@ -3080,7 +3084,7 @@ pub const PackageManager = struct { }; } - this.cache_directory_path = this.allocator.dupe(u8, Path.joinAbsString( + this.cache_directory_path = this.allocator.dupeZ(u8, Path.joinAbsString( Fs.FileSystem.instance.top_level_dir, &.{ "node_modules", diff --git a/src/install/npm.zig b/src/install/npm.zig index 8df30abf62..d705b976dd 100644 --- a/src/install/npm.zig +++ b/src/install/npm.zig @@ -272,7 +272,12 @@ pub const Registry = struct { @as(u32, @truncate(@as(u64, @intCast(@max(0, std.time.timestamp()))))) + 300, )) |package| { if (package_manager.options.enable.manifest_cache) { - PackageManifest.Serializer.save(&package, package_manager.getTemporaryDirectory(), package_manager.getCacheDirectory()) catch {}; + PackageManifest.Serializer.save(&package, package_manager.getTemporaryDirectory(), package_manager.getCacheDirectory()) catch |err| { + if (PackageManager.verbose_install) { + Output.warn("Error caching manifest for {s}: {s}", .{ package_name, @errorName(err) }); + Output.flush(); + } + }; } return PackageVersionResponse{ .fresh = package }; @@ -566,6 +571,14 @@ pub const PackageManifest = struct { return this.pkg.name.slice(this.string_buf); } + pub fn byteLength(this: *const PackageManifest) usize { + var counter = std.io.countingWriter(std.io.null_writer); + const writer = counter.writer(); + + Serializer.write(this, @TypeOf(writer), writer) catch return 0; + return counter.bytes_written; + } + pub const Serializer = struct { pub const version = "bun-npm-manifest-cache-v0.0.2\n"; const header_bytes: string = "#!/usr/bin/env bun\n" ++ version; @@ -657,13 +670,61 @@ pub const PackageManifest = struct { } } - fn writeFile(this: *const PackageManifest, tmp_path: [:0]const u8, tmpdir: std.fs.Dir) !void { - var tmpfile = try tmpdir.createFileZ(tmp_path, .{ - .truncate = true, - }); - defer tmpfile.close(); - const writer = tmpfile.writer(); + fn writeFile(this: *const PackageManifest, tmp_path: [:0]const u8, tmpdir: std.fs.Dir, cache_dir: std.fs.Dir, outpath: [:0]const u8) !void { + // 64 KB sounds like a lot but when you consider that this is only about 6 levels deep in the stack, it's not that much. + var stack_fallback = std.heap.stackFallback(64 * 1024, bun.default_allocator); + + const allocator = stack_fallback.get(); + var buffer = try std.ArrayList(u8).initCapacity(allocator, this.byteLength() + 64); + defer buffer.deinit(); + const writer = &buffer.writer(); try Serializer.write(this, @TypeOf(writer), writer); + // --- Perf Improvement #1 ---- + // Do not forget to buffer writes! + // + // PS C:\bun> hyperfine "bun-debug install --ignore-scripts" "bun install --ignore-scripts" --prepare="del /s /q bun.lockb && del /s /q C:\Users\window\.bun\install\cache" + // Benchmark 1: bun-debug install --ignore-scripts + // Time (mean ± σ): 1.266 s ± 0.284 s [User: 1.631 s, System: 0.205 s] + // Range (min … max): 1.071 s … 1.804 s 10 runs + // + // Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options. + // + // Benchmark 2: bun install --ignore-scripts + // Time (mean ± σ): 3.202 s ± 0.095 s [User: 0.255 s, System: 0.172 s] + // Range (min … max): 3.058 s … 3.371 s 10 runs + // + // Summary + // bun-debug install --ignore-scripts ran + // 2.53 ± 0.57 times faster than bun install --ignore-scripts + // --- Perf Improvement #2 ---- + // GetFinalPathnameByHandle is very expensive if called many times + // We skip calling it when we are giving an absolute file path. + // This needs many more call sites, doesn't have much impact on this location. + var realpath_buf: bun.PathBuffer = undefined; + const path_to_use_for_opening_file = if (Environment.isWindows) + bun.path.joinAbsStringBufZ(PackageManager.instance.temp_dir_path, &realpath_buf, &.{ PackageManager.instance.temp_dir_path, tmp_path }, .auto) + else + tmp_path; + + const file = try bun.sys.File.openat(tmpdir, path_to_use_for_opening_file, std.os.O.CREAT | std.os.O.TRUNC | std.os.O.WRONLY | std.os.O.APPEND, if (Environment.isPosix) 0o664 else 0).unwrap(); + { + errdefer file.close(); + try file.writeAll(buffer.items).unwrap(); + } + if (comptime Environment.isWindows) { + var realpath2_buf: bun.PathBuffer = undefined; + var did_close = false; + errdefer if (!did_close) file.close(); + + const cache_dir_abs = PackageManager.instance.cache_directory_path; + const cache_path_abs = bun.path.joinAbsStringBufZ(cache_dir_abs, &realpath2_buf, &.{ cache_dir_abs, outpath }, .auto); + file.close(); + did_close = true; + try bun.sys.renameat(bun.FD.cwd(), path_to_use_for_opening_file, bun.FD.cwd(), cache_path_abs).unwrap(); + } else { + defer file.close(); + try bun.sys.renameat(bun.toFD(tmpdir), tmp_path, bun.toFD(cache_dir), outpath).unwrap(); + } } pub fn save(this: *const PackageManifest, tmpdir: std.fs.Dir, cache_dir: std.fs.Dir) !void { @@ -678,9 +739,8 @@ pub const PackageManifest = struct { try dest_path_stream_writer.print("{any}.npm-{any}", .{ hex_fmt, hex_timestamp_fmt }); try dest_path_stream_writer.writeByte(0); const tmp_path: [:0]u8 = dest_path_buf[0 .. dest_path_stream.pos - 1 :0]; - try writeFile(this, tmp_path, tmpdir); const out_path = std.fmt.bufPrintZ(&out_path_buf, "{any}.npm", .{hex_fmt}) catch unreachable; - try std.os.renameatZ(tmpdir.fd, tmp_path, cache_dir.fd, out_path); + try writeFile(this, tmp_path, tmpdir, cache_dir, out_path); } pub fn load(allocator: std.mem.Allocator, cache_dir: std.fs.Dir, package_name: string) !?PackageManifest {