Skip to content

Remove build_plan from lockfile to improve perf #1362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
f-f wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Remove build_plan from lockfile to improve perf #1362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fe330b1
Remove build_plan from lockfile to improve perf
f-f Jan 19, 2026
d766aad
Merge branch 'master' into f-f/remove-build-plan-lockfile
f-f Jan 22, 2026
19b5640
Update src/Spago/Command/Fetch.purs
f-f Jan 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 37 additions & 22 deletions src/Spago/Command/Fetch.purs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,17 +417,8 @@ writeNewLockfile reason allTransitiveDeps = do
dependencies <- corePackageDepsOrEmpty packageName package
pure $ FromPath { path, dependencies }

-- For every package that is a `WorkspacePackage`, we just pick up all
-- transitive core and test dependencies of that package from
-- `allTransitiveDeps` and stick them into `core { build_plan }` and
-- `test { build_plan }` respectively.
workspacePackageLockEntries = Map.fromFoldable do
name /\ package <- Config.workspacePackageToLockfilePackage <$> Config.getWorkspacePackages workspace.packageSet
let deps = allTransitiveDeps # Map.lookup name # fromMaybe { core: Map.empty, test: Map.empty }
pure $ name /\ package
{ core { build_plan = Map.keys deps.core }
, test { build_plan = Map.keys deps.test }
}
Config.workspacePackageToLockfilePackage <$> Config.getWorkspacePackages workspace.packageSet

-- For every non-workspace package, we convert it to its respective type of
-- lockfile entry via `packageToLockEntry`.
Expand Down Expand Up @@ -604,27 +595,51 @@ getTransitiveDeps workspacePackage = do
let depsRanges = (map (fromMaybe Config.widestRange) <<< unwrap) `onEachEnv` getWorkspacePackageDeps workspacePackage
{ workspace } <- ask
case workspace.packageSet.lockfile of
-- If we have a lockfile we can just use that - we don't need build a plan, since we store it for every workspace
-- package, so we can just filter out the packages we need.
-- If we have a lockfile we can compute transitive deps from the lockfile data
Right lockfile -> do
case Map.lookup workspacePackage.package.name lockfile.workspace.packages of
Nothing ->
die $ "Package " <> PackageName.print workspacePackage.package.name <> " not found in lockfile"
Just envs ->
pure
{ core: fromBuildPlan envs.core.build_plan
, test: fromBuildPlan envs.test.build_plan
{ core: computeTransitiveDeps envs.core.dependencies
, test: computeTransitiveDeps envs.test.dependencies
}
where
fromBuildPlan bp = Map.union otherPackages workspacePackagesWeNeed
allWorkspacePackages = Map.fromFoldable $ (_.package.name &&& WorkspacePackage) <$> Config.getWorkspacePackages workspace.packageSet

-- Get direct dependencies of a package from the lockfile
getDeps :: PackageName -> Set PackageName
getDeps name = case Map.lookup name lockfile.workspace.packages of
Just pkg -> Set.fromFoldable $ Map.keys $ unwrap pkg.core.dependencies
Nothing -> Set.fromFoldable $ case Map.lookup name lockfile.packages of
Just (FromPath { dependencies }) -> dependencies
Just (FromGit { dependencies }) -> dependencies
Just (FromRegistry { dependencies }) -> dependencies
Nothing -> []

transitiveClosure :: Set PackageName -> Set PackageName
transitiveClosure initial = go initial initial
where
allWorkspacePackages = Map.fromFoldable $ map (\p -> Tuple p.package.name (WorkspacePackage p)) (Config.getWorkspacePackages workspace.packageSet)

isInBuildPlan :: forall v. PackageName -> v -> Boolean
isInBuildPlan name _package = Set.member name bp

workspacePackagesWeNeed = Map.filterWithKey isInBuildPlan allWorkspacePackages
otherPackages = map fromLockEntry $ Map.filterWithKey isInBuildPlan lockfile.packages
go seen new
| Set.isEmpty new = seen
| otherwise =
let
discovered = foldMap getDeps new `Set.difference` seen
in
go (seen <> discovered) discovered

computeTransitiveDeps :: Dependencies -> PackageMap
computeTransitiveDeps deps =
let
transitiveDeps = transitiveClosure $ Set.fromFoldable $ Map.keys $ unwrap deps

isInTransitiveDeps :: forall v. PackageName -> v -> Boolean
isInTransitiveDeps name _ = Set.member name transitiveDeps
in
Map.union
(Map.filterWithKey isInTransitiveDeps allWorkspacePackages)
(map fromLockEntry $ Map.filterWithKey isInTransitiveDeps lockfile.packages)
Comment on lines +612 to +642
Copy link
Collaborator

@fsoikin fsoikin Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand why all of this is necessary.

Before this change, the transitive dependencies used to be initially calculated in the Left branch of this case, and then written to the lockfile, and subsequently lifted from there. This suggests that the logic for computing them already exists in some form inside the Left branch.

Copy link
Member Author

@f-f f-f Jan 23, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, and I tried to unify it, but I now think that the two logics are different enough that it's not worth unifying them: the algo in question is in the Left branch and only in the case of a package set build (since otherwise the solver build is using the solver to get the transitive closure, which is different logic).
The overall algorithm is the same, but when solving a package set build one has effects, caches, and error reporting (since the plan build might fail).
We have none of that when we build a plan from the lockfile, so we can get away with much simpler implementation, even though the logic is overall the same.

Copy link
Collaborator

@fsoikin fsoikin Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see


-- No lockfile, we need to build a plan from scratch, and hit the Registry and so on
Left _ -> case workspace.packageSet.buildType of
Expand Down
9 changes: 4 additions & 5 deletions src/Spago/Config.purs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,8 +575,8 @@ workspacePackageToLockfilePackage { path, package } = Tuple package.name
{ path: case Path.localPart (withForwardSlashes path) of
"" -> "./"
p -> p
, core: { dependencies: package.dependencies, build_plan: mempty }
, test: { dependencies: foldMap _.dependencies package.test, build_plan: mempty }
, core: { dependencies: package.dependencies }
, test: { dependencies: foldMap _.dependencies package.test }
}

type LockfileRecomputeResult =
Expand All @@ -603,13 +603,12 @@ shouldComputeNewLockfile { workspace, workspacePackages } workspaceLock =
]
}
where
eraseBuildPlan = _ { core { build_plan = mempty }, test { build_plan = mempty } }
-- surely this already exists
explainReason flag reason = if flag then Just reason else Nothing

-- Conditions for recomputing the lockfile:
-- 1. the workspace packages should exactly match, except for the needed_by field, which is filled in during build plan construction
workspacesDontMatch = (workspacePackageToLockfilePackage >>> snd <$> workspacePackages) /= (eraseBuildPlan <$> workspaceLock.packages)
-- 1. the workspace packages should exactly match
workspacesDontMatch = (workspacePackageToLockfilePackage >>> snd <$> workspacePackages) /= workspaceLock.packages
-- 2. the extra packages should exactly match
extraPackagesDontMatch = fromMaybe Map.empty workspace.extraPackages /= workspaceLock.extra_packages
-- 3. the package set address needs to match - we have no way to match the package set contents at this point, so we let it be
Expand Down
3 changes: 0 additions & 3 deletions src/Spago/Lock.purs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import Spago.Prelude
import Codec.JSON.DecodeError as CJ.DecodeError
import Data.Codec as Codec
import Data.Codec.JSON as CJ
import Data.Codec.JSON.Common as CJ.Common
import Data.Codec.JSON.Strict as CJS
import Data.Profunctor as Profunctor
import Record as Record
Expand Down Expand Up @@ -62,7 +61,6 @@ type WorkspaceLockPackage =

type WorkspaceLockPackageEnv =
{ dependencies :: Core.Dependencies
, build_plan :: Set PackageName
}

lockfileCodec :: CJ.Codec Lockfile
Expand All @@ -86,7 +84,6 @@ workspaceLockCodec = CJ.named "WorkspaceLock" $ CJS.objectStrict

envCodec = CJ.named "Environment" $ CJS.objectStrict
$ CJS.recordProp @"dependencies" Config.dependenciesCodec
$ CJS.recordProp @"build_plan" (CJ.Common.set PackageName.codec)
$ CJS.record

packageSetCodec :: CJ.Codec PackageSetInfo
Expand Down
8 changes: 1 addition & 7 deletions test-fixtures/build/local-package-set-lockfile/spago.lock.new
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,10 @@
"console",
"effect",
"prelude"
],
"build_plan": [
"console",
"effect",
"prelude"
]
},
"test": {
"dependencies": [],
"build_plan": []
"dependencies": []
}
}
},
Expand Down
8 changes: 1 addition & 7 deletions test-fixtures/build/local-package-set-lockfile/spago.lock.old
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,10 @@
"console",
"effect",
"prelude"
],
"build_plan": [
"console",
"effect",
"prelude"
]
},
"test": {
"dependencies": [],
"build_plan": []
"dependencies": []
}
}
},
Expand Down
12 changes: 3 additions & 9 deletions test-fixtures/lock/1158-doubly-nested-projects/expected-lockfile.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,19 @@
"core": {
"dependencies": [
"prelude"
],
"build_plan": [
"prelude"
]
},
"test": {
"dependencies": [],
"build_plan": []
"dependencies": []
}
},
"root": {
"path": "./",
"core": {
"dependencies": [],
"build_plan": []
"dependencies": []
},
"test": {
"dependencies": [],
"build_plan": []
"dependencies": []
}
}
},
Expand Down
27 changes: 0 additions & 27 deletions test-fixtures/polyrepo.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,13 @@
"package-b",
"package-c",
"prelude"
],
"build_plan": [
"package-b",
"package-c",
"prelude"
]
},
"test": {
"dependencies": [
"console",
"effect",
"prelude"
],
"build_plan": [
"console",
"effect",
"prelude"
]
}
},
Expand All @@ -34,20 +24,11 @@
"dependencies": [
"package-c",
"prelude"
],
"build_plan": [
"package-c",
"prelude"
]
},
"test": {
"dependencies": [
"console"
],
"build_plan": [
"console",
"effect",
"prelude"
]
}
},
Expand All @@ -56,20 +37,12 @@
"core": {
"dependencies": [
"prelude"
],
"build_plan": [
"prelude"
]
},
"test": {
"dependencies": [
"console",
"effect"
],
"build_plan": [
"console",
"effect",
"prelude"
]
}
}
Expand Down
14 changes: 1 addition & 13 deletions test-fixtures/spago-with-maybe.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,10 @@
"effect",
"maybe",
"prelude"
],
"build_plan": [
"console",
"control",
"effect",
"invariant",
"maybe",
"newtype",
"prelude",
"safe-coerce",
"unsafe-coerce"
]
},
"test": {
"dependencies": [],
"build_plan": []
"dependencies": []
}
}
},
Expand Down
8 changes: 1 addition & 7 deletions test-fixtures/spago.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,10 @@
"console",
"effect",
"prelude"
],
"build_plan": [
"console",
"effect",
"prelude"
]
},
"test": {
"dependencies": [],
"build_plan": []
"dependencies": []
}
}
},
Expand Down
23 changes: 1 addition & 22 deletions test/Spago/Lock.purs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import Test.Prelude

import Codec.JSON.DecodeError as CJ.DecodeError
import Data.Map as Map
import Data.Set as Set
import Registry.Range as Range
import Registry.Sha256 as Sha256
import Registry.Version as Version
Expand Down Expand Up @@ -44,26 +43,18 @@ validLockfile =
[ packageTuple "effect" (Just (unsafeFromRight (Range.parse ">=1.0.0 <5.0.0")))
, packageTuple "my-library" Nothing
]
, build_plan: Set.fromFoldable
[ mkPackageName "my-library"
, mkPackageName "effect"
, mkPackageName "prelude"
]
}
, test:
{ dependencies: Dependencies Map.empty
, build_plan: Set.empty
}
, path: "my-app"
}
, packageTuple "my-library"
{ core:
{ dependencies: Dependencies $ Map.fromFoldable [ packageTuple "prelude" Nothing ]
, build_plan: Set.fromFoldable [ mkPackageName "prelude" ]
}
, test:
{ dependencies: Dependencies $ Map.fromFoldable [ packageTuple "console" (Just Config.widestRange) ]
, build_plan: Set.fromFoldable [ mkPackageName "console" ]
}
, path: "my-library"
}
Expand Down Expand Up @@ -133,11 +124,6 @@ validLockfileString =
"my-app": {
"path": "my-app",
"core": {
"build_plan": [
"effect",
"my-library",
"prelude"
],
"dependencies": [
{
"effect": ">=1.0.0 <5.0.0"
Expand All @@ -146,16 +132,12 @@ validLockfileString =
]
},
"test": {
"dependencies": [],
"build_plan": []
"dependencies": []
}
},
"my-library": {
"path": "my-library",
"core": {
"build_plan": [
"prelude"
],
"dependencies": [
"prelude"
]
Expand All @@ -165,9 +147,6 @@ validLockfileString =
{
"console": "*"
}
],
"build_plan": [
"console"
]
}
}
Expand Down
Loading