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

Fix CircleCI cache #4171

Merged
merged 1 commit into from
Dec 30, 2018
Merged

Fix CircleCI cache #4171

merged 1 commit into from
Dec 30, 2018

Conversation

kubawerlos
Copy link
Contributor

Currently on 2.12 we only save cache, never restore it :(

PR split into 2 commits:

  • first one to save cache when having content to cache - what's important with a different key
  • second one to restore it - the reason for split is to see after second commit the build time change

@kubawerlos kubawerlos changed the title Fix CircleCI Fix CircleCI cache Dec 28, 2018
@@ -22,5 +16,12 @@ jobs:
- run: php composer.phar global show hirak/prestissimo -q || php composer.phar global require --no-interaction --no-progress --optimize-autoloader hirak/prestissimo
- run: php composer.phar install --optimize-autoloader --no-interaction --no-progress --no-suggest
- run: php composer.phar info -D | sort

- save_cache:
key: source-v1-{{ .Branch }}-{{ .Revision }}
Copy link
Member

Choose a reason for hiding this comment

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

don't cache by revision, please, too detailed.

homebrew we can cache by php version, but we have only one currently, so we can cache always
composer's vendor we can cache by php version (also not relevant) and PR target branch (2.12/2.13/master/3.0),
but even if we would not, most of the deps would be cached properly, as 2.12 vs 3.0 has very little deps change.

So, even just adding restore_cache would be enough, no need for interpolated cache key

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, do you know how to speed up brew update? It takes usually 5 minutes out of 8 for the build.

Copy link
Member

Choose a reason for hiding this comment

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

no, i'm not using mac

- ~/Library/Caches/Homebrew
- restore_cache:
keys:
- source-v1-{{ checksum "composer.json" }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@keradus what about this key?

Copy link
Member

Choose a reason for hiding this comment

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

no hard opinion on this one

ker@dus:~/github/FriendsOfPHP/PHP-CS-Fixer λ composer info | wc -l
64
ker@dus:~/github/FriendsOfPHP/PHP-CS-Fixer λ git diff 2.12..3.0 composer.json
diff --git a/composer.json b/composer.json
index 5df4718f..b42e19aa 100644
--- a/composer.json
+++ b/composer.json
@@ -20,7 +20,7 @@
         "composer/semver": "^1.4",
         "composer/xdebug-handler": "^1.2",
         "doctrine/annotations": "^1.2",
-        "php-cs-fixer/diff": "^1.3",
+        "php-cs-fixer/diff": "^2.0",
         "symfony/console": "^3.4.17 || ^4.1.6",
         "symfony/event-dispatcher": "^3.0 || ^4.0",
         "symfony/filesystem": "^3.0 || ^4.0",

if we have 64 package installed, do we really want to not use cache of them, if one package is different ? installing 63 packages from cache + one fresh vs 64 fresh...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, and then it will cache them and reuse again - bear in mind that CircleCI will not save the cache when there already is anything saved under the key - take a look here in "Save Cache" action - 1 month old cache, impossible to track what is there.

Copy link
Member

Choose a reason for hiding this comment

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

oh damn it !
ok, then your proposal of using checksum makes perfect sense !

why source-v1 prefix ? what does it stand for ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no idea - it's from the docs - never thought about it. Example seems to cache source and we would cache dependencies and Homebrew - what name would you propose? maybe simply cache?

@kubawerlos
Copy link
Contributor Author

Okay, summarising:

  • updating xcode from 9.0 to 10.0.1 reduced build time (precisely brew update) by 2.5~3 minutes
  • fixing cache gave a very little boost (composer installing prestissimo and rest of deps took up to 20 seconds, after fix almost 0)

@keradus keradus added this to the 2.12.5 milestone Dec 30, 2018
@keradus keradus added RTM Ready To Merge and removed RTM Ready To Merge labels Dec 30, 2018
@keradus
Copy link
Member

keradus commented Dec 30, 2018

Thank you @kubawerlos.

@keradus keradus merged commit ab6c68a into PHP-CS-Fixer:2.12 Dec 30, 2018
keradus added a commit that referenced this pull request Dec 30, 2018
This PR was merged into the 2.12 branch.

Discussion
----------

Fix CircleCI cache

Currently on `2.12` we only save cache, never restore it :(

PR split into 2 commits:
 - first one to save cache when having content to cache - what's important with a different key
 - second one to restore it - the reason for split is to see after second commit the build time change

Commits
-------

ab6c68a Fix CircleCI cache
@kubawerlos kubawerlos deleted the fix-circle-ci-cache branch December 30, 2018 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants