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

sitemap/traversal: Use compiled regex for performance improvement #2502

Merged
merged 1 commit into from
Sep 17, 2021
Merged

sitemap/traversal: Use compiled regex for performance improvement #2502

merged 1 commit into from
Sep 17, 2021

Conversation

davebarrau
Copy link
Contributor

I found when building a large site (Gitlab's Handbook) with a lot of pages, the flame graph was getting stuck quite a bit in the @store.resources.find block in sitemap/extensions/traversal.rb.

With the change in this MR I was able to decrease the build time from ~3m30s to ~2m20s, a ~33% saving for this specific use case.

@tdreyno
Copy link
Member

tdreyno commented Sep 16, 2021

Fantastic find. Thank you.

@tdreyno tdreyno enabled auto-merge (squash) September 16, 2021 21:35
@davebarrau
Copy link
Contributor Author

Fantastic find. Thank you.

No worries! Looks like the ubuntu-ruby-2.4 check got stuck or failed to report in. Can you re-run it?

@tdreyno tdreyno merged commit 316655d into middleman:4.x Sep 17, 2021
@davebarrau davebarrau deleted the 4.x-sitemap-traversal-regex-improvement branch September 18, 2021 00:11
@davebarrau
Copy link
Contributor Author

@tdreyno Hi Thomas! Thanks again for merging this. Is it possible to release a 4.4.1 with this PR in it? :) Or, when do you expect the next release to land?

@tdreyno
Copy link
Member

tdreyno commented Nov 2, 2021

Done

@davebarrau
Copy link
Contributor Author

Done

Thanks @tdreyno! I think the gem might have the same issue 4.3.9 had (filesize 4.5 KB rather than 5.5 KB):

4.4.1 - November 02, 2021 (4.5 KB)
4.4.0 - June 16, 2021 (5.5 KB)
4.3.11 - September 15, 2020 (5.5 KB)
4.3.10 - September 10, 2020 (5.5 KB)
4.3.9 - September 09, 2020 (4.5 KB) yanked

Apologies for being a pain!

@tdreyno
Copy link
Member

tdreyno commented Nov 3, 2021

Fixed. Forgot that my windows machine is borked for gem pushing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants