Skip to content

Use .dagger-root to improve module load performance on large monorepos (#8496, #8499)#8953

Closed
idlsoft wants to merge 1 commit into
dagger:mainfrom
idlsoft:slow-module-load
Closed

Use .dagger-root to improve module load performance on large monorepos (#8496, #8499)#8953
idlsoft wants to merge 1 commit into
dagger:mainfrom
idlsoft:slow-module-load

Conversation

@idlsoft
Copy link
Copy Markdown
Contributor

@idlsoft idlsoft commented Nov 14, 2024

dagger looks for the git repo root when it loads a module.
This has a performance impact, especially on large monorepos.
This change will allow to limit the scope of the module by adding a .dagger-root file.

@idlsoft idlsoft marked this pull request as ready for review November 14, 2024 17:13
@jedevc
Copy link
Copy Markdown
Contributor

jedevc commented Nov 14, 2024

cc @sipsma @helderco

@github-actions
Copy link
Copy Markdown
Contributor

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@helderco
Copy link
Copy Markdown
Contributor

helderco commented Dec 5, 2024

Should open an issue proposing this change before opening a PR, but I don't think this is the right approach.

You mention #8496 (in the title), but I think the better solution there is to remove those includes by default, and have users be explicit if they want to add anything outside their module root.

Additionally with #6627, more stuff should be excluded by default, even within your module like a heavy node_modules, .yarn_cache, or vendor. I commented on the other issue that you reference (in the title) how this really affects performance. See #8499 (comment).

We could do both of these soon, now that #8818 has been merged. It's just a matter of bandwidth and priorities right now.

This way the module's context directory will continue to be limited at the git repo root, which we make assumptions about all over the code base, without affecting performance since if you don't add any files outside the module's root (in dagger.json), it'll just find the path of the git root, not load any files from it.

@idlsoft
Copy link
Copy Markdown
Contributor Author

idlsoft commented Dec 5, 2024

Should open an issue proposing this change before opening a PR, but I don't think this is the right approach.

This is definitely a stopgap solution. I was originally thinking about adding settings to dagger.json but this was just easier.

This way the module's context directory will continue to be limited at the git repo root, which we make assumptions about all over the code base, without affecting performance since if you don't add any files outside the module's root (in dagger.json), it'll just find the path of the git root, not load any files from it.

Right, I think most of the time is being spent in trying to locate all the '**/...' patterns.
As long as this doesn't happen implicitly the performance should not suffer.

In my example the module had one simple root, but ideally it'd be nice to be able to specify a list of directories to include in the module context, so you could have the main module + a few libraries with common build logic.
In a monorepo these libraries would typically reside somewhere near the top of the git repo, and modules using them closer to the actual code that needs to be built.

@helderco
Copy link
Copy Markdown
Contributor

helderco commented Dec 6, 2024

ideally it'd be nice to be able to specify a list of directories to include in the module context, so you could have the main module + a few libraries with common build logic.

You can already do this with the include field in dagger.json. Use a relative path. It's just that go modules implicitly include all go files in the repo, that's what needs to change. Other SDKs don't suffer from this. Then, users relying on this implicitness will have to explicitly add the right patterns to their dagger.json files.

However, if possible, a better solution is to put the common build logic in its own module and install it as a dependency where needed.

@github-actions
Copy link
Copy Markdown
Contributor

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 9, 2025

This PR was closed because it has been stalled for 7 days with no activity.

@github-actions github-actions Bot closed this Jan 9, 2025
@idlsoft idlsoft deleted the slow-module-load branch December 30, 2025 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants