feat: dedup & cached build performance improvements #3935
Conversation
mndeveci
changed the title
samcli/lib/build/build_graph.py
Outdated
| return self.functions[0].get_build_dir(artifact_root_dir) | ||
| build_dir = self.functions[0].get_build_dir(artifact_root_dir) | ||
| if is_experimental_enabled(ExperimentalFlag.BuildImprovements22) and len(self.functions) > 1: | ||
| build_dir = f"{build_dir}-Shared" |
There was a problem hiding this comment.
How is this suffix "Shared" determined?
There was a problem hiding this comment.
When determining the artifacts directory that we will have our resultant build in, before we would copy all the contents in the same code URI (technically the same build definition) over and over. Now if there are multiple functions with the same build definition we will just put them into one single shared artifacts directory.
There was a problem hiding this comment.
I see, putting up that comment above the line and also making the suffix a constant string thats defined and importable would be good.
|
I wonder if we can recommend this option/flag based on the runtime...if the project is using node, we can probably add a hint to use this in the CLI. |
These changes aren't necessarily runtime-specific except for the accelerate related changes which are only applicable to Python and esbuild. |
| self._validate_functions() | ||
| return self.functions[0].get_build_dir(artifact_root_dir) | ||
| build_dir = self.functions[0].get_build_dir(artifact_root_dir) | ||
| if is_experimental_enabled(ExperimentalFlag.BuildPerformance) and len(self.functions) > 1: |
There was a problem hiding this comment.
Add message to customers about this change and add a link to a doc about all the changes that this includes. Will do this in another PR.
There was a problem hiding this comment.
Discussion for review session has been captured in the comments above. The main focus would be to release this feature in the upcoming releases and then construct a corresponding documentation link explaining on the experimental features. With that customers can better understand the improvements.
* create symlink during cached build & use first built folder for de-dup builds * add experimental flag and unit tests * add adl use case * create osutil function * create osutil function * update integration tests * format files * Rename feature flag * Add comment and make shared constant Co-authored-by: Daniel Mil <mladan@amazon.com> Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com> Co-authored-by: Qingchuan Ma <69653965+qingchm@users.noreply.github.com>
5 participants
Which issue(s) does this change fix?
#3093
#2688
Why is this change necessary?
Previously, SAM CLI improved build performance by introducing de-duplicated & cached builds. These improvements are using copying previously built resources to skip some of the build operations. But runtimes like
nodejsmight contain lots of files insidenode_modulesfolder which makes copy operation slow.How does it address the issue?
In order to address this issue, this PR has following improvements;
CodeUri) we will be building into a shared folder and point all of the functions to that folder rather than copying first built function into rest of them.What side effects does this change have?
Certain operating systems might not support symlinking correctly. Because of that, we are checking if any errors are thrown with symlink operation, if so we will fall-back to copy.
This feature will also be released behind a feature flag to address some issues before marking it as GA.
Mandatory Checklist
PRs will only be reviewed after checklist is complete
make prpassesmake update-reproducible-reqsif dependencies were changedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.