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.

Sign up for GitHub

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

Jump to bottom

.ci/deps.sh: Fix dartsdk caching #1777

Closed
wants to merge 2 commits into from
Closed

.ci/deps.sh: Fix dartsdk caching #1777

wants to merge 2 commits into from

Conversation

jayvdb
Copy link
Member

@jayvdb jayvdb commented May 26, 2017

-v always fails as that is not a valid command line argument.
Use --version instead.

Fixes #1776

@jayvdb jayvdb mentioned this pull request May 26, 2017
Merged
@underyx
Copy link
Member

underyx commented May 26, 2017

darksdk in commit message should be dartsdk

@underyx
Copy link
Member

underyx commented May 26, 2017

Any idea why tests fail now?

@jayvdb
Copy link
Member Author

jayvdb commented May 26, 2017

My guess is the caching of only dartsdk/bin is the problem.

@jayvdb jayvdb force-pushed the fix-dart-sdk-caching branch from a9e0986 to 6c4a37d Compare May 26, 2017 09:00
@jayvdb jayvdb force-pushed the fix-dart-sdk-caching branch from 6c4a37d to 55090ef Compare May 26, 2017 09:22
@jayvdb
Copy link
Member Author

jayvdb commented May 26, 2017

So now the question is, does having a larger cache perform better than always doing a fetch from a reliable host and an unzip.

#1769 will make it easier to answer that question, but I guess @li3n3 might know the answer.

The usual advice from CI providers seems to be to not cache things like this.

@jayvdb jayvdb changed the title .ci/deps.sh: Fix darksdk caching .ci/deps.sh: Fix dartsdk caching May 26, 2017
@jayvdb jayvdb force-pushed the fix-dart-sdk-caching branch from 55090ef to a81d3c0 Compare May 26, 2017 12:30
@jayvdb
Copy link
Member Author

jayvdb commented May 26, 2017

Always hard to prove anything with CI performance, without doing a lot of silly identical test runs, but ..

So based on one run, we should be removing the dart caching.
But of course, it is illogical that deps.sh would take longer when it has less to do.

@jayvdb jayvdb mentioned this pull request May 26, 2017
Closed
@gitmate-bot
Copy link
Collaborator

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

4 similar comments
@gitmate-bot
Copy link
Collaborator

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

@gitmate-bot
Copy link
Collaborator

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

@gitmate-bot
Copy link
Collaborator

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

@gitmate-bot
Copy link
Collaborator

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

@jayvdb jayvdb force-pushed the fix-dart-sdk-caching branch from a81d3c0 to e54258e Compare June 24, 2017 13:41
jayvdb added 2 commits June 24, 2017 20:44
@jayvdb
.ci/deps.sh: Fix dartsdk caching
e54258e 
`-v` always fails as that is not a valid command line argument.
Use existence of the unzipped files to test if it is installed.

Fixes coala#1776
@jayvdb
Force uncached build
c16111b
@jayvdb
Copy link
Member Author

jayvdb commented Jun 25, 2017

Rebased https://circleci.com/gh/jayvdb/coala-bears/772 and restoring the cache with dartsdk is 50s again,
and restoring cache without dartsdk https://circleci.com/gh/jayvdb/coala-bears/775 is 39s .

This time, the cached sdk deps.sh is 32s and the uncached sdk deps.sh is 34s.

Adding timing to uncached, the wget is 0.63s (real), and the unzip is 0.34s. By using wget -q its time goes down to 0.469s. i.e. installing the uncached dart sdk costs is easily less than 1s operation, assuming no network problems.

But https://circleci.com/gh/jayvdb/coala-bears/777 also has a 50s cache restore step, so it is still possible to have higher cache restore times without dart sdk in the cache. And https://circleci.com/gh/coala/coala-bears/6625 had a 36s cache restore, so low cache restores are possible with the dart sdk bin in the cache.

@gitmate-bot
Copy link
Collaborator

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

@gitmate-bot
Copy link
Collaborator

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

@sils
Copy link
Member

sils commented Jul 16, 2017

how does c16111b force an uncached build? 😕

@sils
Copy link
Member

sils commented Jul 16, 2017

Given the timings it seems it's pretty irrelevant if we have caching for this or not, right? I'm ok with either of those, caching is kind of a complexity but also helps consistency just caching everything.

@jayvdb
Copy link
Member Author

jayvdb commented May 26, 2019

Irrelevant due to #2910

@jayvdb jayvdb closed this May 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
process/wip review carefully! size/S status/STALE
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

deps.sh: Don't download dart-sdk if it exists (again)
4 participants
@jayvdb @underyx @gitmate-bot @sils