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

Add prebuilt image for python #127

Merged
merged 7 commits into from
May 1, 2021
Merged

Add prebuilt image for python #127

merged 7 commits into from
May 1, 2021

Conversation

ybbbby
Copy link
Collaborator

@ybbbby ybbbby commented Apr 9, 2021

Python image is built by Bazel from grpc/grpc, and can be further optimized.
After built, bone structure/binary is extracted to a new image to shrink the final image's size.
The build procedure mimic the init-containers' clone, build and run steps.
I have verified the prebuilt image on benchmark-prod reaching the same stage as regular multi-stage images.
The image size:

30b5098c98b6   30 seconds ago   /bin/sh -c #(nop) COPY file:1288b1d07ba39958…   52B       
2cc577ed0a38   22 hours ago     /bin/sh -c #(nop) COPY file:2119411c1149daf8…   4.99MB  

So approximately 8.43MB would be pushed to gcr.io.
Using prebuilt image, we can complete the performance test within 50s while we need 7m32s to finish using multi-stage images. Thus, we can save approximately 6m40s.

@ybbbby ybbbby marked this pull request as ready for review April 13, 2021 01:33
@ybbbby ybbbby self-assigned this Apr 13, 2021
@codeblooded codeblooded changed the title Add prebuilt image fro python Add prebuilt image for python Apr 15, 2021
@wanlin31
Copy link
Collaborator

wanlin31 commented Apr 19, 2021

I think for this great work you have done, it will be nice to let reviewers know that:
(usually what I did, hopefully helpful :) )

(1) You have verified with benchmark-prod (deployed controller) or benchmark-dev1 (deployed controller or local controller run against cluster) using this image, and reach to the same stage as regular images. (I usually have this statement in the comment for the previous images, it is up to you how you want to verify it, but at least people know you have verified it)
(2) What is the size for the binary we need to push each time to the repositories.
I have done the analysis of this one, we will push ~8.43MB each time to gcr.io, looks great.

64227fbc6206   10 minutes ago   /bin/sh -c #(nop) COPY file:838ec40eee1c3cf3…   52B       
ed81bd397642   10 minutes ago   /bin/sh -c #(nop) COPY file:016ebc82536415ae…   8.43MB 

(3) How much is the time we saved for using pre-built image.

I have not went through cxx for these step in the comments is because it is part of the investigation and have been verified and reported several times, and for go, I have not yet mark the pr as ready, I will probably do the same thing as I described.

@wanlin31
Copy link
Collaborator

This apply to all the images, I leave it here so I won't flood all your PRs. :)

@wanlin31 wanlin31 self-requested a review April 19, 2021 22:39
@ybbbby
Copy link
Collaborator Author

ybbbby commented Apr 20, 2021

I think for this great work you have done, it will be nice to let reviewers know that:
(usually what I did, hopefully helpful :) )

(1) You have verified with benchmark-prod (deployed controller) or benchmark-dev1 (deployed controller or local controller run against cluster) using this image, and reach to the same stage as regular images. (I usually have this statement in the comment for the previous images, it is up to you how you want to verify it, but at least people know you have verified it)
(2) What is the size for the binary we need to push each time to the repositories.
I have done the analysis of this one, we will push ~8.43MB each time to gcr.io, looks great.

64227fbc6206   10 minutes ago   /bin/sh -c #(nop) COPY file:838ec40eee1c3cf3…   52B       
ed81bd397642   10 minutes ago   /bin/sh -c #(nop) COPY file:016ebc82536415ae…   8.43MB 

(3) How much is the time we saved for using pre-built image.

I have not went through cxx for these step in the comments is because it is part of the investigation and have been verified and reported several times, and for go, I have not yet mark the pr as ready, I will probably do the same thing as I described.

Thanks for pointing out!

@paulosjca paulosjca self-requested a review April 23, 2021 22:32
@wanlin31
Copy link
Collaborator

I think Paulo already mentioned things I want to talk about. LGTM.

@ybbbby ybbbby requested a review from wanlin31 April 30, 2021 18:52
@ybbbby ybbbby merged commit ae02306 into grpc:master May 1, 2021
@ybbbby ybbbby deleted the pythonPrebuilt branch June 1, 2021 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants