From a61f1f8cdd8712af5df317c1921cd9a6adc2d991 Mon Sep 17 00:00:00 2001 From: Jason Prasad Date: Thu, 4 Jun 2026 22:52:59 -0400 Subject: [PATCH 1/3] Snapshot: pin window capture to deterministic 1x bitmapImageRepForCachingDisplay inherits the host window's backingScaleFactor, so on a Retina (2x) display the snapshot came out 800x1200 instead of the logical 400x600, making output depend on which machine the daemon ran on. Build the NSBitmapImageRep manually at the point bounds so capture is 1x and reproducible across machines. Co-Authored-By: Claude Opus 4.8 --- Sources/PreviewsMacOS/Snapshot.swift | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/Sources/PreviewsMacOS/Snapshot.swift b/Sources/PreviewsMacOS/Snapshot.swift index dd3a2f6..57bc600 100644 --- a/Sources/PreviewsMacOS/Snapshot.swift +++ b/Sources/PreviewsMacOS/Snapshot.swift @@ -26,13 +26,27 @@ public enum Snapshot { guard bounds.width > 0, bounds.height > 0 else { throw SnapshotError.captureFailed } - // Note: bitmapImageRepForCachingDisplay produces 1x images. Off-screen headless - // windows aren't associated with a display, so backingScaleFactor is 1.0 anyway. - // To capture at a specific scale, create an NSBitmapImageRep manually with scaled - // pixel dimensions. (pointfreeco/swift-snapshot-testing has the same limitation.) - guard let bitmapRep = contentView.bitmapImageRepForCachingDisplay(in: bounds) else { + // Pin the raster to a deterministic 1x (one pixel per point). bitmapImageRepForCachingDisplay + // inherits the host window's backingScaleFactor — 2x on a Retina display — which made the + // snapshot's pixel dimensions depend on which machine the daemon ran on. Building the rep + // manually with pixel dimensions equal to the point bounds keeps output reproducible. + guard + let bitmapRep = NSBitmapImageRep( + bitmapDataPlanes: nil, + pixelsWide: Int(bounds.width.rounded()), + pixelsHigh: Int(bounds.height.rounded()), + bitsPerSample: 8, + samplesPerPixel: 4, + hasAlpha: true, + isPlanar: false, + colorSpaceName: .deviceRGB, + bytesPerRow: 0, + bitsPerPixel: 0 + ) + else { throw SnapshotError.captureFailed } + bitmapRep.size = bounds.size contentView.cacheDisplay(in: bounds, to: bitmapRep) switch format { From 4c410231924311298e7829ef91ce618e4ef72b8f Mon Sep 17 00:00:00 2001 From: Jason Prasad Date: Thu, 4 Jun 2026 22:53:09 -0400 Subject: [PATCH 2/3] JIT: hot-reload non-leaf files via single-module incremental split The structural split compiled the bulk into a prebuilt stable module that the hot file imported. That is one-directional, so a non-leaf hot file (one the bulk references, e.g. a previewed view used by another preview) failed with "cannot find in scope" when the bulk compiled without it. Detect the non-leaf case (the stable-module compile fails) and compile the whole target module incrementally instead: the editable overlay plus the other sources under one module name, where the Swift driver recompiles only the hot file and reuses the bulk objects. References resolve in both directions. Because the overlay then uses the target's own module name, its @Observable DesignTimeStore would re-register across generations, so non-leaf builds respawn the agent each structural edit. hotReloadStructural now exercises this on the JIT daemon. Its sync marker moves from "Compiled:" (only the non-JIT recompile logs it) to "Reloaded", which both daemon kinds log on a successful structural edit. Co-Authored-By: Claude Opus 4.8 --- Sources/PreviewsCore/Compiler.swift | 73 +++++++++++++++++ Sources/PreviewsCore/PreviewSession.swift | 81 ++++++++++++++++--- .../JITStructuralReloader.swift | 9 ++- Tests/MCPIntegrationTests/MacOSMCPTests.swift | 4 +- 4 files changed, 153 insertions(+), 14 deletions(-) diff --git a/Sources/PreviewsCore/Compiler.swift b/Sources/PreviewsCore/Compiler.swift index 04105ea..c9d2cf5 100644 --- a/Sources/PreviewsCore/Compiler.swift +++ b/Sources/PreviewsCore/Compiler.swift @@ -259,6 +259,79 @@ public actor Compiler { return StableModule(moduleName: moduleName, modulesDir: moduleDir, objectPath: objectFile) } + /// Per-module incremental build directory, reused across edits so the driver's + /// `.swiftdeps` records and the unchanged-file objects persist between compiles. + private var incrementalDirs: [String: URL] = [:] + + /// Compile the whole target module incrementally: the editable `overlaySource` plus the + /// target's other `bulkFiles`, all under one `moduleName`. The Swift driver recompiles only + /// what changed — the overlay alone on a body edit, the overlay plus its dependents on an + /// interface edit — and reuses the rest from the persistent build dir. Returns the overlay's + /// object and the bulk objects (in `bulkFiles` order). This is the non-leaf structural split: + /// the bulk references the edited file, so a one-directional prebuilt stable module cannot be + /// used, but a single module resolves references in both directions. + public func compileModuleIncremental( + overlaySource: String, + bulkFiles: [URL], + moduleName: String, + extraFlags: [String] = [] + ) async throws -> (overlayObject: URL, bulkObjects: [URL]) { + let dir: URL + if let existing = incrementalDirs[moduleName] { + dir = existing + } else { + dir = workDir.appendingPathComponent("incremental-\(moduleName)", isDirectory: true) + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + incrementalDirs[moduleName] = dir + } + + let overlayFile = dir.appendingPathComponent("overlay.swift") + try overlaySource.write(to: overlayFile, atomically: true, encoding: .utf8) + let overlayObject = dir.appendingPathComponent("overlay.o") + + // The output-file-map keys must match the command-line paths exactly, or the driver + // disables incremental ("no swiftDeps file") and recompiles everything every edit. + var fileMap: [String: [String: String]] = [ + "": ["swift-dependencies": dir.appendingPathComponent("master.swiftdeps").path], + overlayFile.path: [ + "object": overlayObject.path, + "swift-dependencies": dir.appendingPathComponent("overlay.swiftdeps").path, + ], + ] + var bulkObjects: [URL] = [] + for (index, file) in bulkFiles.enumerated() { + let object = dir.appendingPathComponent("bulk_\(index).o") + bulkObjects.append(object) + fileMap[file.path] = [ + "object": object.path, + "swift-dependencies": dir.appendingPathComponent("bulk_\(index).swiftdeps").path, + ] + } + let mapFile = dir.appendingPathComponent("output-file-map.json") + let mapData = try JSONSerialization.data(withJSONObject: fileMap, options: [.sortedKeys]) + try mapData.write(to: mapFile) + + var args: [String] = [ + swiftcPath, + "-incremental", + "-emit-object", + "-parse-as-library", + "-target", targetTriple, + "-sdk", sdkPath, + "-module-name", moduleName, + "-Onone", + "-gnone", + "-module-cache-path", moduleCachePath.path, + "-output-file-map", mapFile.path, + ] + args += extraFlags + args += [overlayFile.path] + args += bulkFiles.map(\.path) + + try await run(args) + return (overlayObject, bulkObjects) + } + // MARK: - Private @discardableResult diff --git a/Sources/PreviewsCore/PreviewSession.swift b/Sources/PreviewsCore/PreviewSession.swift index 20cc5f3..2f544ae 100644 --- a/Sources/PreviewsCore/PreviewSession.swift +++ b/Sources/PreviewsCore/PreviewSession.swift @@ -26,6 +26,11 @@ public struct JITRenderBuild: Sendable { /// Binary dynamic libraries (the target's `-F`/`-framework` dependency frameworks) the /// agent must `dlopen` so their symbols resolve. Empty for standalone. public let dylibPaths: [URL] + /// Require a freshly respawned agent for this render instead of a new generation on the + /// live one. The non-leaf incremental split compiles under the target's own (stable) module + /// name, so its `@Observable DesignTimeStore` would re-register across generations in one + /// process; a fresh process each structural edit sidesteps the duplicate registration. + public let requiresFreshAgent: Bool public init( objectPath: URL, @@ -35,7 +40,8 @@ public struct JITRenderBuild: Sendable { literals: [LiteralEntry], supportObjectPaths: [URL] = [], archivePaths: [URL] = [], - dylibPaths: [URL] = [] + dylibPaths: [URL] = [], + requiresFreshAgent: Bool = false ) { self.objectPath = objectPath self.imagePath = imagePath @@ -45,6 +51,7 @@ public struct JITRenderBuild: Sendable { self.supportObjectPaths = supportObjectPaths self.archivePaths = archivePaths self.dylibPaths = dylibPaths + self.requiresFreshAgent = requiresFreshAgent } } @@ -237,19 +244,45 @@ public actor PreviewSession { var supportObjectPaths: [URL] = [] var archivePaths: [URL] = [] var dylibPaths: [URL] = [] + var requiresFreshAgent = false if let (ctx, bulk) = splitContext { - let stable = try await stableModule(for: bulk, context: ctx) - supportObjectPaths = [stable.objectPath] archivePaths = Self.dependencyArchives(in: ctx.compilerFlags) if let runtimeArchive = try await Toolchain.compilerRuntimeArchivePath() { archivePaths.append(URL(fileURLWithPath: runtimeArchive)) } dylibPaths = Self.dependencyDylibs(in: ctx.compilerFlags) - objectPath = try await compiler.compileObject( - source: generated.source, - moduleName: "PreviewEdit_\(ctx.moduleName)_\(Self.uniqueModuleToken())", - extraFlags: ["-I", stable.modulesDir.path] + ctx.compilerFlags - ) + + if let stable = try await stableModuleIfLeaf(for: bulk, context: ctx) { + supportObjectPaths = [stable.objectPath] + objectPath = try await compiler.compileObject( + source: generated.source, + moduleName: "PreviewEdit_\(ctx.moduleName)_\(Self.uniqueModuleToken())", + extraFlags: ["-I", stable.modulesDir.path] + ctx.compilerFlags + ) + } else { + // Non-leaf: the bulk references the edited file, so it cannot be prebuilt as a + // one-directional stable module. Compile the whole module incrementally with the + // overlay in-module (no `@testable import`); only the hot file recompiles per edit. + let overlay = BridgeGenerator.generateCombinedSource( + originalSource: source, + closureBody: preview.closureBody, + previewIndex: previewIndex, + platform: platform, + traits: traits, + renderOutputPath: imagePath.path, + designTimeValuesPath: valuesPath.path, + stableModuleImport: nil + ) + let built = try await compiler.compileModuleIncremental( + overlaySource: overlay.source, + bulkFiles: bulk, + moduleName: ctx.moduleName, + extraFlags: ctx.compilerFlags + ) + supportObjectPaths = built.bulkObjects + objectPath = try Self.uniqueObjectCopy(of: built.overlayObject) + requiresFreshAgent = true + } } else { objectPath = try await compiler.compileObject( source: generated.source, @@ -269,7 +302,8 @@ public actor PreviewSession { literals: generated.literals, supportObjectPaths: supportObjectPaths, archivePaths: archivePaths, - dylibPaths: dylibPaths + dylibPaths: dylibPaths, + requiresFreshAgent: requiresFreshAgent ) lastJITBuild = build return build @@ -342,6 +376,35 @@ public actor PreviewSession { /// Return the stable module for `bulk`, reusing the cached one when no bulk file has /// changed (the common case: repeated edits to the hot preview file). Rebuilds only when /// a bulk file's modification date changes, so the per-edit compile stays narrow. + private var bulkIsNonLeaf = false + + /// The stable half of the leaf split, or nil when the bulk references the hot file (non-leaf) + /// and so cannot compile without it. Cached per session: once the bulk fails to compile on its + /// own, every later edit takes the incremental whole-module path. A genuine bulk error surfaces + /// again from that path, so the fallback never hides it. + private func stableModuleIfLeaf( + for bulk: [URL], context ctx: BuildContext + ) async throws -> Compiler.StableModule? { + if bulkIsNonLeaf { return nil } + do { + return try await stableModule(for: bulk, context: ctx) + } catch is CompilationError { + bulkIsNonLeaf = true + return nil + } + } + + /// Copy the incremental build's reused `overlay.o` (a stable path) to a unique path, so each + /// structural edit presents a distinct `objectPath`. The reloader keys its literal fast path on + /// object-path identity, which a stable path would falsely trigger. + private static func uniqueObjectCopy(of object: URL) throws -> URL { + let dest = object.deletingLastPathComponent() + .appendingPathComponent("overlay-\(uniqueModuleToken()).o") + try? FileManager.default.removeItem(at: dest) + try FileManager.default.copyItem(at: object, to: dest) + return dest + } + private func stableModule( for bulk: [URL], context ctx: BuildContext ) async throws -> Compiler.StableModule { diff --git a/Sources/PreviewsJITLink/JITStructuralReloader.swift b/Sources/PreviewsJITLink/JITStructuralReloader.swift index effb368..d96466b 100644 --- a/Sources/PreviewsJITLink/JITStructuralReloader.swift +++ b/Sources/PreviewsJITLink/JITStructuralReloader.swift @@ -25,7 +25,7 @@ public actor JITStructuralReloader: StructuralReloader { return } - let session = try nextSession() + let session = try nextSession(forceFresh: build.requiresFreshAgent) for dylib in build.dylibPaths { try session.addDylib(path: dylib.path) } @@ -49,9 +49,10 @@ public actor JITStructuralReloader: StructuralReloader { /// The session to link this edit into: a fresh `JITDylib` on the live agent while under /// the cap, otherwise a freshly respawned agent (replacing the old one, whose `deinit` - /// kills its process). The first edit and each post-cap edit start a new agent. - private func nextSession() throws -> JITSession { - if let session, generation < generationCap { + /// kills its process). The first edit, each post-cap edit, and any `forceFresh` edit (the + /// non-leaf incremental split, which reuses the target's stable module name) start a new agent. + private func nextSession(forceFresh: Bool) throws -> JITSession { + if let session, !forceFresh, generation < generationCap { generation += 1 try session.newGeneration() return session diff --git a/Tests/MCPIntegrationTests/MacOSMCPTests.swift b/Tests/MCPIntegrationTests/MacOSMCPTests.swift index 12664fd..968fac9 100644 --- a/Tests/MCPIntegrationTests/MacOSMCPTests.swift +++ b/Tests/MCPIntegrationTests/MacOSMCPTests.swift @@ -525,7 +525,9 @@ struct MacOSMCPTests { to: URL(fileURLWithPath: filePath), atomically: false, encoding: .utf8) // CI swiftc on cold caches is slow; AGENTS.md notes daemon startup alone is 5–10s. - try await server.awaitStderrContains("Compiled:", timeout: .seconds(90)) + // "Reloaded" matches both daemon kinds: the JIT agent logs "Reloaded (JIT agent)!" and + // the non-JIT recompile logs "Reloaded!". Both fire only after a successful structural edit. + try await server.awaitStderrContains("Reloaded", timeout: .seconds(90)) _ = try await server.awaitSnapshotChange( sessionID: sessionID, baseline: baseline, timeout: .seconds(15) ) From 014f17cdc9a15a7bfa4b26039d7484d8d5bda987 Mon Sep 17 00:00:00 2001 From: Jason Prasad Date: Thu, 4 Jun 2026 22:55:17 -0400 Subject: [PATCH 3/3] JIT C++: fix lookupInitialized lock and anon-mapper bounds lookupInitialized read session->initialized outside the lock with no re-check, so two threads could both pass the guard and double-initialize the JITDylib (and the bool itself was a data race). Hold the lock across the check and the init so the flag is only touched under the lock. PreviewsAnonymousMapper::prepare and ::initialize decremented the upper_bound iterator with no bounds check, which is undefined behavior if the reservation map is empty or the address precedes all reservations. Guard r == begin(): prepare returns nullptr, initialize reports an Expected error. Co-Authored-By: Claude Opus 4.8 --- .../PreviewsJITLinkCxx/PreviewsJITLinkCxx.cpp | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/Sources/PreviewsJITLinkCxx/PreviewsJITLinkCxx.cpp b/Sources/PreviewsJITLinkCxx/PreviewsJITLinkCxx.cpp index 378d95e..867d213 100644 --- a/Sources/PreviewsJITLinkCxx/PreviewsJITLinkCxx.cpp +++ b/Sources/PreviewsJITLinkCxx/PreviewsJITLinkCxx.cpp @@ -106,6 +106,9 @@ class PreviewsAnonymousMapper : public llvm::orc::MemoryMapper { char *prepare(llvm::orc::ExecutorAddr addr, size_t contentSize) override { std::lock_guard lock(mutex); auto r = reservations.upper_bound(addr); + if (r == reservations.begin()) { + return nullptr; + } --r; return r->second.workingBuf + (addr - r->first); } @@ -117,6 +120,11 @@ class PreviewsAnonymousMapper : public llvm::orc::MemoryMapper { { std::lock_guard lock(mutex); auto r = reservations.upper_bound(ai.MappingBase); + if (r == reservations.begin()) { + return onInitialized(llvm::createStringError( + llvm::inconvertibleErrorCode(), + "initialize: no reservation covers mapping base")); + } --r; base = r->second.workingBuf + (ai.MappingBase - r->first); } @@ -273,13 +281,17 @@ struct previewsmcp_jit_session { namespace { llvm::Expected lookupInitialized(previewsmcp_jit_session *session, const char *symbol_name) { - if (!session->initialized) { + { static std::mutex initMutex; std::lock_guard lock(initMutex); - if (auto err = session->jit->initialize(*session->jd)) { - return std::move(err); + // Re-check under the lock: the flag is read and written only here, so two + // threads cannot both pass the check and double-initialize the JITDylib. + if (!session->initialized) { + if (auto err = session->jit->initialize(*session->jd)) { + return std::move(err); + } + session->initialized = true; } - session->initialized = true; } return session->jit->lookup(*session->jd, symbol_name); }