Skip to content

Commit

Permalink
PackageManager: Make lookup lazy again
Browse files Browse the repository at this point in the history
We want to avoid loading the whole cache whenever lookup is called,
as most invocations of dub should result in a call to lookup,
since dependency resolution is much rarer than building with an
existing dub.selections.json.
  • Loading branch information
Geod24 committed Jun 5, 2024
1 parent 5b7b3f9 commit 0c64803
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 28 deletions.
88 changes: 60 additions & 28 deletions source/dub/packagemanager.d
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,27 @@ class PackageManager {
*/
Location[] m_repositories;
/**
* Whether `refresh` has been called or not
* Whether `refreshLocal` / `refreshCache` has been called or not
*
* Dub versions because v1.31 eagerly scan all available repositories,
* leading to slowdown for the most common operation - `dub build` with
* already resolved dependencies.
* From v1.31 onward, those locations are not scanned eagerly,
* unless one of the function requiring eager scanning does,
* such as `getBestPackage` - as it needs to iterate the list
* of available packages.
* User local cache can get pretty large, and we want to avoid our build
* time being dependent on their size. However, in order to support
* local packages and overrides, we were scanning the whole cache prior
* to v1.39.0 (although attempts at fixing this behavior were made
* earlier). Those booleans record whether we have been semi-initialized
* (local packages and overrides have been loaded) or fully initialized
* (all caches have been scanned), the later still being required for
* some API (e.g. `getBestPackage` or `getPackageIterator`).
*/
bool m_initialized;
enum InitializationState {
/// No `refresh*` function has been called
none,
/// `refreshLocal` has been called
partial,
/// `refreshCache` (and `refreshLocal`) has been called
full,
}
/// Ditto
InitializationState m_state;
}

/**
Expand Down Expand Up @@ -191,8 +201,10 @@ class PackageManager {
* A `Package` if one was found, `null` if none exists.
*/
protected Package lookup (in PackageName name, in Version vers) {
if (!this.m_initialized)
this.refresh();
// This is the only place we can get away with lazy initialization,
// since we know exactly what package and version we want.
// However, it is also the most often called API.
this.ensureInitialized(InitializationState.partial);

if (auto pkg = this.m_internal.lookup(name, vers))
return pkg;
Expand Down Expand Up @@ -635,9 +647,8 @@ class PackageManager {
*/
int delegate(int delegate(ref Package)) getPackageIterator()
{
// See `m_initialized` documentation
if (!this.m_initialized)
this.refresh();
// This API requires full knowledge of the package cache
this.ensureInitialized(InitializationState.full);

int iterator(int delegate(ref Package) del)
{
Expand Down Expand Up @@ -974,8 +985,7 @@ symlink_exit:
// As we iterate over `localPackages` we need it to be populated
// In theory we could just populate that specific repository,
// but multiple calls would then become inefficient.
if (!this.m_initialized)
this.refresh();
this.ensureInitialized(InitializationState.full);

path.endsWithSlash = true;
auto pack = this.load(path);
Expand Down Expand Up @@ -1006,8 +1016,7 @@ symlink_exit:
// As we iterate over `localPackages` we need it to be populated
// In theory we could just populate that specific repository,
// but multiple calls would then become inefficient.
if (!this.m_initialized)
this.refresh();
this.ensureInitialized(InitializationState.full);

path.endsWithSlash = true;
Package[]* packs = &m_repositories[type].localPackages;
Expand Down Expand Up @@ -1050,29 +1059,52 @@ symlink_exit:
{
if (refresh)
logDiagnostic("Refreshing local packages (refresh existing: true)...");
this.refresh_(refresh);
else
logDiagnostic("Scanning local packages...");

this.refreshLocal(refresh);
this.refreshCache(refresh);
}

void refresh()
{
this.refresh_(false);
logDiagnostic("Scanning local packages...");
this.refreshLocal(false);
this.refreshCache(false);
}

private void refresh_(bool refresh)
/// Private API to ensure a level of initialization
private void ensureInitialized(InitializationState state)
{
if (!refresh)
logDiagnostic("Scanning local packages...");
if (this.m_state >= state)
return;
if (state == InitializationState.partial)
this.refreshLocal(false);
else
this.refresh();
}

/// Refresh pay-as-you-go: Only load local packages, not the full cache
private void refreshLocal(bool refresh) {
foreach (ref repository; this.m_repositories)
repository.scanLocalPackages(refresh, this);

this.m_internal.scan(this, refresh);
foreach (ref repository; this.m_repositories)
repository.scan(this, refresh);
foreach (ref repository; this.m_repositories) {
auto existing = refresh ? null : repository.fromPath;
foreach (path; repository.searchPath)
repository.scanPackageFolder(path, this, existing);
repository.loadOverrides(this);
}
if (this.m_state < InitializationState.partial)
this.m_state = InitializationState.partial;
}

/// Refresh the full cache, a potentially expensive operation
private void refreshCache(bool refresh)
{
foreach (ref repository; this.m_repositories)
repository.loadOverrides(this);
this.m_initialized = true;
repository.scan(this, refresh);
this.m_state = InitializationState.full;
}

alias Hash = ubyte[];
Expand Down
18 changes: 18 additions & 0 deletions source/dub/test/other.d
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,21 @@ unittest
dub.loadPackage();
assert(dub.project.hasAllDependencies());
}

// Check that a simple build does not lead to the cache being scanned
unittest
{
scope dub = new TestDub((scope FSEntry fs) {
// This should never be read
fs.writePackageFile("b", "1.0.0", `poison`);
fs.writePackageFile("b", "1.1.0", `poison`);
// Dependency resolution may trigger scan, so we need a selections file
fs.writeFile(TestDub.ProjectPath ~ "dub.json",
`{ "name": "a", "dependencies": {"b":"~>1.0"}}`);
fs.writeFile(TestDub.ProjectPath ~ "dub.selections.json",
`{"fileVersion":1,"versions":{"b":"1.0.4"}}`);
fs.writePackageFile("b", "1.0.4", `{"name":"b","version":"1.0.4"}`);
});
dub.loadPackage();
assert(dub.project.hasAllDependencies());
}

0 comments on commit 0c64803

Please sign in to comment.