Skip to content

Conversation

@ascorbic
Copy link
Contributor

@ascorbic ascorbic commented Nov 24, 2025

Summary

The sitemap generator uses git to get the last modified time. Currently this execs git for every path, which is very slow – currently accounting for half of the build time. This PR optimises this by instead getting all of the last modified times with a single git log command, and caching them in a Map. This reduces the time on my local machine from ~5 minutes to 400 ms, and in production from around 9 mins to ~800ms, cutting build time approximately in half.

Screenshots (optional)

Documentation checklist

name: Build
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
ENABLE_LAST_MOD_IN_SITEMAP: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not needed anymore, as it's fast

Copy link
Collaborator

@kodster28 kodster28 Nov 24, 2025

Choose a reason for hiding this comment

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

We should wait to remove this until we've confirmed that across a couple builds. Making assumptions about what does / doesn't affect the build time is sorta how we got into the sitemap generation (and it's build time regression) situation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I did benchmark a few scenarios locally though, and removing it here means it runs in this PR, which it wouldn't otherwise.

@github-actions
Copy link
Contributor

This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:

Pattern Owners
/.github/ @cloudflare/pcx-content-engineering, @kodster28
*.ts @cloudflare/pcx-content-engineering, @kodster28

@github-actions
Copy link
Contributor

@kodster28
Copy link
Collaborator

cc: @colbywhite

Copy link
Collaborator

@colbywhite colbywhite left a comment

Choose a reason for hiding this comment

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

@ascorbic , Thanks for the input. I'm currently working on this exact logic but was implementing it slightly differently to account for edge cases. Let's merge this for now and I'll refactor it later today.

@colbywhite colbywhite merged commit 4d95086 into production Nov 24, 2025
11 checks passed
@colbywhite colbywhite deleted the faster-sitemap-lastmod branch November 24, 2025 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants