-
Notifications
You must be signed in to change notification settings - Fork 1k
Fix #607: Improve build times drastically #684
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
Conversation
Per @dragos's suggestion, this commit implements a new approach to generate table of contents that uses internally populated Jekyll data structures to speed up listing and matching posts that are from the same category. As categories are only populated by posts and our current tutorials are pages, this commits turns pages into posts and makes sure that their permalinks correspond to the current ones. Build times are now around two minutes, see the following profile information from Jekyll: ``` jvican in /data/rw/code/scala/scala.github.com [11:02:40] > $ bundle exec jekyll serve --profile [±drone-integration ●●●] Configuration file: /data/rw/code/scala/scala.github.com/_config.yml Configuration file: /data/rw/code/scala/scala.github.com/_config.yml Source: /data/rw/code/scala/scala.github.com Destination: /data/rw/code/scala/scala.github.com/_site Incremental build: disabled. Enable with --incremental Generating... Filename | Count | Bytes | Time -------------------------------------------------------------------+-------+-----------+------- _layouts/overview-large.html | 164 | 26568.57K | 92.778 _includes/toc-large.txt | 164 | 23156.00K | 88.934 _layouts/tutorial.html | 199 | 3005.98K | 1.350 _layouts/overview.html | 37 | 1190.86K | 0.953 _includes/tutorial-toc.txt | 199 | 267.11K | 0.455 es/overviews/index.md | 1 | 46.66K | 0.443 _includes/localized-overview-index.txt | 1 | 46.49K | 0.443 _includes/tutorial-tour-list.txt | 200 | 244.89K | 0.432 _includes/header.txt | 439 | 2114.25K | 0.347 _layouts/cheatsheet.html | 6 | 225.08K | 0.217 _includes/cheatsheet-sidebar.txt | 6 | 42.70K | 0.207 style/index.md | 1 | 68.00K | 0.184 _includes/footer.txt | 449 | 855.91K | 0.175 tutorials/index.md | 1 | 54.34K | 0.168 _includes/footerbar.txt | 458 | 767.51K | 0.116 _includes/topbar.txt | 419 | 822.04K | 0.110 _includes/pager.txt | 363 | 27.10K | 0.079 _layouts/sip.html | 29 | 626.83K | 0.079 _layouts/guides-index.html | 4 | 55.73K | 0.069 _includes/disqus.txt | 386 | 324.89K | 0.044 _layouts/sip-landing.html | 10 | 171.17K | 0.038 _includes/toc.txt | 66 | 17.62K | 0.034 _layouts/default.html | 261 | 29198.78K | 0.030 _includes/allsids.txt | 10 | 17.88K | 0.019 _layouts/index.html | 7 | 261.62K | 0.011 _includes/sips-topbar.txt | 39 | 31.73K | 0.008 sips/pending/_posts/2016-01-11-static-members.md | 1 | 10.23K | 0.004 _includes/index-header.txt | 7 | 53.96K | 0.004 _includes/cheatsheet-header.txt | 6 | 57.92K | 0.003 _layouts/contribute.html | 1 | 21.40K | 0.003 _includes/frontpage-footer.txt | 9 | 15.29K | 0.003 sips/sip-list.md | 1 | 3.19K | 0.003 sips/completed/_posts/2016-06-25-trailing-commas.md | 1 | 10.62K | 0.002 _layouts/news-coursera.html | 1 | 26.31K | 0.002 _layouts/frontpage.html | 1 | 13.17K | 0.002 _layouts/page.html | 1 | 14.69K | 0.002 _layouts/glossary.html | 1 | 65.08K | 0.002 _layouts/search.html | 1 | 12.77K | 0.002 _includes/contributing-header.txt | 2 | 12.13K | 0.001 sips/minutes-list.md | 1 | 0.76K | 0.001 ja/overviews/collections/concrete-mutable-collection-classes.md | 1 | 13.02K | 0.001 overviews/collections/concrete-mutable-collection-classes.md | 1 | 11.09K | 0.001 ja/overviews/collections/overview.md | 1 | 8.73K | 0.001 _includes/header-coursera.txt | 1 | 5.21K | 0.001 _includes/frontpage-header.txt | 1 | 4.94K | 0.001 es/overviews/parallel-collections/concrete-parallel-collections.md | 1 | 13.97K | 0.001 overviews/parallel-collections/concrete-parallel-collections.md | 1 | 12.57K | 0.001 sips/sip-template.md | 1 | 4.30K | 0.001 overviews/collections/concrete-immutable-collection-classes.md | 1 | 14.17K | 0.001 _includes/glossary-header.txt | 1 | 10.10K | 0.001 done in 123.422 seconds. ```
Given the necessity of this change for all our current PRs, I would propose to merge this as soon as possible 😄. Review by @heathermiller. |
@jvican, from a cursory glance, this looks good. Did you make sure to generate and double check that links are not broken and that the permalinks for the tour is still the same? I believe there's a way to run a thing that double checks that links aren't broken (search the issue tracker, I think @jarrodwb added it recently) |
In particular, I'd ask you to double check that the links to the translated pages still work. (From the looks of things, should be fine, but you never know with jekyll + liquid) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please just confirm to me that you've run the broken link checker thing, and that links to stuff like translations and permalinks aren't broken by this.
I've double-checked that manually but for sure I would prefer a tool to do it for me, so I'll use it before giving my final approval to these changes. |
It seems that |
By the way, I've just pushed some changes to all the translations. Double checking I found that the index for translations was not correctly propagated/rebuilt for every language. Now it is. However, I've found something even more impressing... For some reason, build times are now down to 89 seconds locally instead of ~120. This liquid thingie is pretty weird -- this change is not supposed to decrease build times, it's supposed to increase it. But anyway -- cool thing is that now it's even faster. @heathermiller My double checks also extend to translations, which have always the prefix of the language prepended to the permalink and therefore is correct, because they are always expected in |
How is |
|
If it's running on GitHub, it would make sense that |
|
Anyway, merging this as-is. Let's see if |
Indeed there's still an issue with the |
I'm trying to identify what's the culprit. Keep you posted. |
I cannot reproduce locally, in any way. But I think I found the issue. Travis kills tut because it's very memory intensive and is spawning scala compiler instances like crazy. I've opened a new PR to replace Travis by Drone. |
Per @dragos's suggestion, this commit implements a new approach to
generate table of contents that uses internally populated Jekyll data
structures to speed up listing and matching posts that are from the same
category.
As categories are only populated by posts and our current tutorials are
pages, this commits turns pages into posts and makes sure that their
permalinks correspond to the current ones.
Build times are now around two minutes, see the following profile
information from Jekyll: