Skip to content

Conversation

@NicoK
Copy link
Contributor

@NicoK NicoK commented Aug 19, 2019

What is the purpose of the change

_includes/sidenav.html parses through pages_by_language over and over again trying to find children when building the (recursive) side navigation. By doing this once with a group_by, we can gain considerable savings in building the docs via ./build_docs.sh without any change to the generated HTML pages:

before: ~54s
after: ~37s

Next PR in this series: #9489

Brief change log

Building on top of #9444, this PR adds:

  • use group_by to create an easier-iterable array for determining page children

Verifying this change

I verified the changes in the generated HTML pages (nothing changed).

NicoK added 8 commits August 15, 2019 00:04
…pages

Starting command tags with "{%-" will drop all whitespace to the left and ending
with "-%}" will drop all whitespace to the right (including newlines!).
Code like the following would otherwise create quite some unnecessary
whitespace:

  {% if parent_id %}
    {% assign parent_id = current[0].nav-parent_id %}
  {% else %}
    {% break %}
  {% endif %}
Jekyll requires liquid and only optionally uses liquid-c if available. The
latter uses natively-compiled code and reduces generation time by ~5% for me.
This seems to come with a much nicer code highlighting.
Jekyll requires sass but can optionally also use a C-based implementation
provided by sassc. Although we do not use sass directly, there may be some
indirect use inside jekyll. It doesn't seem to hurt to upgrade here.
./build_docs.sh -i previously did not only enable incremental documentation
building while serving the docs, it also enabled a 'liveserve' mode that
automatically reloaded pages in the browser when they changed. This is based
on the 'hawkins' module which is not (yet) compatible with jekyll 4.0 which we
need to (significantly) improve build times.

This disables the liveserve mode and remove the hawkins module until a new
version is available.
This significantly reduces the build times, on my machine from 140s to 47s!
_includes/sidenav.html parses through pages_by_language over and over again
trying to find children when building the (recursive) side navigation. By doing
this once with a group_by, we can gain considerable savings in building the
docs via `./build_docs.sh` without any change to the generated HTML pages:

before: ~54s
after: ~37s
@flinkbot
Copy link
Collaborator

flinkbot commented Aug 19, 2019

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Automated Checks

Last check on commit be58c37 (Mon Aug 26 11:14:31 UTC 2019)

Warnings:

  • Documentation files were touched, but no .zh.md files: Update Chinese documentation or file Jira ticket.

Mention the bot in a comment to re-run the automated checks.

Review Progress

  • ❓ 1. The [description] looks good.
  • ❓ 2. There is [consensus] that the contribution should go into to Flink.
  • ❓ 3. Needs [attention] from.
  • ❓ 4. The change fits into the overall [architecture].
  • ❓ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.

Details
The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commands
The @flinkbot bot supports the following commands:

  • @flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
  • @flinkbot approve all to approve all aspects
  • @flinkbot approve-until architecture to approve everything until architecture
  • @flinkbot attention @username1 [@username2 ..] to require somebody's attention
  • @flinkbot disapprove architecture to remove an approval you gave earlier

@flinkbot
Copy link
Collaborator

flinkbot commented Aug 19, 2019

CI report:

@StephanEwen
Copy link
Contributor

Looks good, merging this...

@asfgit asfgit closed this in c64e167 Aug 26, 2019
NicoK added a commit that referenced this pull request Nov 14, 2019
_includes/sidenav.html parses through pages_by_language over and over again
trying to find children when building the (recursive) side navigation. By doing
this once with a group_by, we can gain considerable savings in building the
docs via `./build_docs.sh` without any change to the generated HTML pages:

This closes #9487
NicoK added a commit that referenced this pull request Nov 14, 2019
_includes/sidenav.html parses through pages_by_language over and over again
trying to find children when building the (recursive) side navigation. By doing
this once with a group_by, we can gain considerable savings in building the
docs via `./build_docs.sh` without any change to the generated HTML pages:

This closes #9487
NicoK added a commit that referenced this pull request Nov 14, 2019
_includes/sidenav.html parses through pages_by_language over and over again
trying to find children when building the (recursive) side navigation. By doing
this once with a group_by, we can gain considerable savings in building the
docs via `./build_docs.sh` without any change to the generated HTML pages:

This closes #9487
NicoK added a commit that referenced this pull request Nov 14, 2019
_includes/sidenav.html parses through pages_by_language over and over again
trying to find children when building the (recursive) side navigation. By doing
this once with a group_by, we can gain considerable savings in building the
docs via `./build_docs.sh` without any change to the generated HTML pages:

This closes #9487
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.

4 participants