8000 Add strip to cog builds by 8W9aG · Pull Request #1902 · replicate/cog · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add strip to cog builds #1902

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

Merged
merged 8 commits into from
Sep 3, 2024
Merged

Add strip to cog builds #1902

merged 8 commits into from
Sep 3, 2024

Conversation

8W9aG
Copy link
Contributor
@8W9aG 8W9aG commented Aug 23, 2024
  • Strip out all unnecessary debug symbols shipped by prebuilt shared objects.
  • Careful not to touch the cpython binaries they don’t like strip, but they can be handled
    separately with flags.
  • This speeds up load times of the shared objects, especially when it comes to PyTorch.

* Strip out all unnecessary debug symbols shipped
by prebuilt shared objects.
* Careful not to touch the cpython binaries they
don’t like strip, but they can be handled
separately with flags.
* This speeds up load times of the shared objects,
especially when it comes to PyTorch.
@nevillelyh
Copy link
Contributor

RUN creates a new layer with the same files striped, so it might significantly increase image size. Is it possible to do them in the same RUN pip install step?

@8W9aG
Copy link
Contributor Author
8W9aG commented Aug 23, 2024

RUN creates a new layer with the same files striped, so it might significantly increase image size. Is it possible to do them in the same RUN pip install step?

Yeh great idea I'll do it there

8W9aG added 2 commits August 23, 2024 16:45
* Do this so the entire operation happens in one
layer of docker.
* Include —-no-cache-dir as a way of preventing
non-running related code from entering the docker
Signed-off-by: Will Sackfield <sackfield@replicate.com>
Copy link
Contributor
@technillogue technillogue left a comment

Choose a reason for hiding this comment

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

this looks reasonable, but it would be best to do this as a separate release from other optimizations so it can be rolled back if anything breaks.

it might also be nice to have a way to disable this, since it naturally makes segfaults a lot harder to debug

@8W9aG
Copy link
Contributor Author
8W9aG commented Aug 27, 2024

this looks reasonable, but it would be best to do this as a separate release from other optimizations so it can be rolled back if anything breaks.

it might also be nice to have a way to disable this, since it naturally makes segfaults a lot harder to debug

I do agree however a problem we face is that we need to build the base images as one shot, so some stuff needs to be done at the same time, and some stuff can be punted on. In terms of what we need to rebuild the base images:

We can wait on:

I can add a flag to disable the strip command if you like?

@8W9aG 8W9aG requested a review from technillogue August 27, 2024 20:35
8W9aG added 4 commits August 28, 2024 15:56
* Allow users to specify whether they want to
strip the shared objects for a faster runtime.
Signed-off-by: Will Sackfield <sackfield@replicate.com>
@8W9aG 8W9aG merged commit 2620838 into main Sep 3, 2024
15 checks passed
@8W9aG 8W9aG deleted the strip-installed-libs branch September 3, 2024 20:43
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.

3 participants
0