Skip to content
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.

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

Optimization for the performance of addModules(), addExplodedModules(), and addFlattenModules() #1770

Closed
wants to merge 3 commits into from

Conversation

edithwuly
Copy link
Contributor

@edithwuly edithwuly commented May 19, 2023

Summary

Reduce the number of times to traverse buildpack bits. Save about 27% time spent on addExplodedModules().
Call NToLayerTar in goroutine. Save more than half time spent on addFlattenModules().

Description

Before

Each buildpack bits will be traversed three times: download, calculate diffID and stream to daemon.
NToLayerTar in addFlattenModules() is called for each flattenModule sequentially.

After

The diffID of buildpack will calculated while it is downloaded and saved as a tarfile.
NToLayerTar is called in goroutine, referring to addExplodedModules().

Profiling Result

command average time spent on addExplodedModules() before average time spent on addExplodedModules() after ratio
pack build demo-app --builder cnbs/sample-builder:bionic -p ../samples/java/maven --buildpack paketobuildpacks/java:latest 3.804439914s 2.746395149s 0.72189115
command average time spent on addFlattenModules() before average time spent on addFlattenModules() after ratio
pack builder create cnbs/sample-builder:alpine --config builder.toml --flatten 7.761502674s 3.751928196s 0.48340229
# builder.toml
# Buildpacks to include in builder
[[buildpacks]]
uri = "../samples/buildpacks/hello-processes"

[[buildpacks]]
# Packaged buildpacks to include in builder;
# the "hello-universe" package contains the "hello-world" and "hello-moon" buildpacks
uri = "docker://paketobuildpacks/java:latest"

# Order used for detection
[[order]]
    # This buildpack will create a process type "sys-info" to display runtime information
    [[order.group]]
    id = "samples/hello-processes"
    version = "0.0.1"

# Stack that will be used by the builder
[stack]
id = "io.buildpacks.samples.stacks.bionic"
# This image is used at runtime
run-image = "cnbs/sample-stack-run:bionic"
# This image is used at build-time
build-image = "cnbs/sample-stack-build:bionic"

Documentation

  • Should this change be documented?
    • Yes, see #___
    • No

Related

Resolves #___

@edithwuly edithwuly requested review from a team as code owners May 19, 2023 10:34
@github-actions github-actions bot added the type/enhancement Issue that requests a new feature or improvement. label May 19, 2023
@github-actions github-actions bot added this to the 0.30.0 milestone May 19, 2023
Signed-off-by: edithwuly <[email protected]>
Copy link
Member

@natalieparellano natalieparellano left a comment

Choose a reason for hiding this comment

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

Awesome work @edithwuly! Just a note to maintainers, we might want to hold this PR until we can update the benchmark suite here to show the impact of this change by using a larger buildpack (the samples one is only a few KB it turns out).

@natalieparellano
Copy link
Member

As per usual it seems Codecov is being a bit too picky here... I wonder if there is a way we can get it to ignore error checks, at least on i/o operations 🤔

@github-actions github-actions bot modified the milestones: 0.31.0, 0.30.0 May 23, 2023
@edithwuly edithwuly force-pushed the optimizing branch 2 times, most recently from 76595b6 to 51f2a82 Compare May 23, 2023 12:26
@edithwuly edithwuly changed the title Draft: reduce the number of times to traverse buildpack bits Draft: Optimization for the performance of addExplodedModules() and addFlattenModule() May 23, 2023
@edithwuly edithwuly changed the title Draft: Optimization for the performance of addExplodedModules() and addFlattenModule() Draft: Optimization for the performance of addExplodedModules() and addFlattenModules() May 23, 2023
Signed-off-by: edithwuly <[email protected]>
@natalieparellano natalieparellano changed the title Draft: Optimization for the performance of addExplodedModules() and addFlattenModules() Optimization for the performance of addModules(), addExplodedModules(), and addFlattenModules() May 23, 2023
@natalieparellano
Copy link
Member

@buildpacks/platform-maintainers could you please merge this PR first: #1773

Copy link
Member

@jjbustamante jjbustamante left a comment

Choose a reason for hiding this comment

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

Great job @edithwuly! Thanks for parallelize the logic for the flatten feature!! ❤️

@natalieparellano
Copy link
Member

Summarizing offline discussion - I think we're planning to fold this PR into some work that @jjbustamante is doing (as there will likely be a merge conflict)

@jjbustamante
Copy link
Member

I am closing this PR based on the latest conversation regarding this feature and the after the RFC is approved, we will remove most the current implementation and this is not going to be valid anymore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flatten feature type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants