theia-ide / theia-apps

Theia applications examples - docker images, desktop apps, packagings
Apache License 2.0
1.04k stars 345 forks source link

Reduce image size #425

Closed ericcitaire closed 3 years ago

ericcitaire commented 3 years ago

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

ericcitaire commented 3 years ago

@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
marcdumais-work commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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

stale[bot] commented 3 years ago

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!

ericcitaire commented 3 years ago

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