Skip to content
This repository has been archived by the owner on Oct 5, 2022. It is now read-only.

Reduce image size #425

Closed
wants to merge 3 commits into from
Closed

Reduce image size #425

wants to merge 3 commits into from

Conversation

ericcitaire
Copy link

Cleanup package managers caches (apt, go, pip) in order to get smaller layers.

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

@ericcitaire thank you for the contribution, as part of the changes have you identified with metrics if in fact the total image size was actually reduced? It'd be good to have a comparison in image size before and after the changes.

Please note that optimization was attempted before without any significant impact on image size: #389 (comment).

@ericcitaire
Copy link
Author

@ericcitaire thank you for the contribution, as part of the changes have you identified with metrics if in fact the total image size was actually reduced? It'd be good to have a comparison in image size before and after the changes.

Please note that optimization was attempted before without any significant impact on image size: #389 (comment).

@vince-fugnitto Here is a comparison after building it locally (size reduced by ~270MB) :

ericcitaire/theia-full                   latest              f6626beb4b45        4 hours ago         8.32GB
theiaide/theia-full                      latest              6c8e07141969        5 weeks ago         8.59GB

Signed-off-by: Eric Citaire <eric.citaire@zenika.com>
Signed-off-by: Eric Citaire <eric.citaire@zenika.com>
Signed-off-by: Eric Citaire <eric.citaire@zenika.com>
@marcdumais-work
Copy link
Member

@vince-fugnitto Here is a comparison after building it locally (size reduced by ~270MB) :

ericcitaire/theia-full                   latest              f6626beb4b45        4 hours ago         8.32GB
theiaide/theia-full                      latest              6c8e07141969        5 weeks ago         8.59GB

Thanks @ericcitaire .

In my local tests, it looks-like the apt cleanups contribute little to the size reduction above, despite being many LoC - in the order of a tenth, or ~30MB. If confirmed, I'm not sure it's worth the visual pollution, if we can instead get ~90% of the benefits keeping only the pip cache and go tool cleanups. WDYT?

@ericcitaire
Copy link
Author

@vince-fugnitto Here is a comparison after building it locally (size reduced by ~270MB) :

ericcitaire/theia-full                   latest              f6626beb4b45        4 hours ago         8.32GB
theiaide/theia-full                      latest              6c8e07141969        5 weeks ago         8.59GB

Thanks @ericcitaire .

In my local tests, it looks-like the apt cleanups contribute little to the size reduction above, despite being many LoC - in the order of a tenth, or ~30MB. If confirmed, I'm not sure it's worth the visual pollution, if we can instead get ~90% of the benefits keeping only the pip cache and go tool cleanups. WDYT?

@marcdumais-work : I agree that APT cleanups add a lot of noise to the Dockerfile.

An idea : what if we create a little script at the start of the Dockerfile that would wrap all the APT commands and cleanup?

Something like this :

/usr/bin/install-package:

#!/bin/bash

apt-get update && \
    apt-get --no-install-recommends -y -q install "$@" && \
    apt-get clean && \
    apt-get autoremove -y && \
    rm -rf /var/cache/apt/* && \
    rm -rf /var/lib/apt/lists/* && \
    rm -rf /tmp/*

Note : the option --no-install-recommends is not used everywhere for now. It might cause some problems. Or it might just reduce the image size a bit more. :)

@marcdumais-work
Copy link
Member

An idea : what if we create a little script at the start of the Dockerfile that would wrap all the APT commands and cleanup?

@ericcitaire Just to make sure I understand, are you suggesting that instead of directly installing apt dependencies as is done ATM, we instead would install such packages through this new script, that also does the cleanup behind the scene, without adding noise to the Dockerfile? e.g.:

RUN /usr/bin/install-package $dep1 $dep2 ...

If so, it's an interesting idea, but I am not sure it's worth the trouble for a sub 0.5% decrease in image size. Remember that the apps in this repo are meant as examples, and so a smaller image size at any cost is not always the goal - the Dockerfile being easier to understand can be the correct trade-off in some cases.

Can you amend the PR to remove the apt cleanup commit? I think the two other commits are nice, low-hanging fruits, image size-wise.

@marcdumais-work
Copy link
Member

@ericcitaire Are you ok if someone picks-up the two commits we'd like to merge, preserving your authorship, adding probably a third commit on top to adapt to the latest master branch?

@ericcitaire
Copy link
Author

@marcdumais-work Yes, no problem at all. Even if you don't preserve authorship, I don't mind.

@stale
Copy link

stale bot commented Jan 30, 2021

This contribution has been automatically marked as stale due to inactivity, and it will be closed if no further activity occurs. Thank you for contributing to Theia!

@stale stale bot added the stale label Jan 30, 2021
@ericcitaire
Copy link
Author

Closing this. Feel free to cherry-pick any changes you want.

@ericcitaire ericcitaire closed this Feb 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants