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

tpl: Add findRE template func #2048

Closed
wants to merge 1 commit into from
Closed

tpl: Add findRE template func #2048

wants to merge 1 commit into from

Conversation

digitalcraftsman
Copy link
Member

No description provided.

@digitalcraftsman digitalcraftsman changed the title tpl: Add findRE templace func tpl: Add findRE template func Apr 5, 2016
@bep
Copy link
Member

bep commented Apr 5, 2016

What is the performance implications of adding this to a medium sized Hugo site?

@digitalcraftsman
Copy link
Member Author

How do you define medium sized? And the more complex the regexp is the longer it takes. Can you suggest a website for a benchmark with a usecase for a regexp?

@bep
Copy link
Member

bep commented Apr 5, 2016

With the regexp in the example. Would be valuable to see what adding that to the single template on a couple of hundred pages blog adds in time.

@digitalcraftsman
Copy link
Member Author

What do you think if I add the scrollspy example from the docs into the templates for the Hugo pages? Hugo's docs consist of around generated 180 pages.

@bep
Copy link
Member

bep commented Apr 5, 2016

Sure, and you don't have to use the result, just invoke the regexp for every page.

@digitalcraftsman
Copy link
Member Author

The setup: I copied the content of a file from the docs in the default archetype. Next, I wrote a small bash script that generates 400 new content files (is this medium sized?). Finally, I build the site. The benchmark of course does only include the build time of the website not the generation of the content files.

This are the results without regexp:

time ./hugo -s docs

Started building site
0 draft content
0 future content
573 pages created
0 non-page files copied
0 paginator pages created
56 tags created
0 groups created
in 4837 ms
./hugo -s docs  14,46s user 0,42s system 303% cpu 4,901 total

and with the complete scrollspy regexp:

time ./hugo -s docs

Started building site
0 draft content
0 future content
573 pages created
0 non-page files copied
0 paginator pages created
0 groups created
56 tags created
in 4880 ms
./hugo -s docs  14,44s user 0,45s system 301% cpu 4,945 total

As you can see the difference if marginal with such a simple regexp.

@moorereason
Copy link
Contributor

@digitalcraftsman,
I'm curious what the difference would be if you used the existing regexpCache used by replaceRE.

@digitalcraftsman
Copy link
Member Author

Thanks for pointing me to the reCache. I tested it a couple of times and got on average the following result:

Started building site
0 draft content
0 future content
573 pages created
0 non-page files copied
0 paginator pages created
0 groups created
56 tags created
in 4948 ms
./hugo -s docs  14,31s user 0,48s system 295% cpu 5,015 total

Under the line a good improvement of 40ms .

@bep
Copy link
Member

bep commented Apr 7, 2016

Do the 400 content files you generated have matches for your regexp?

@digitalcraftsman
Copy link
Member Author

Yes they have. The default archetype contaited content with multiple level-two headers. Therefore, each generated page triggered the regexp with multiple matches.

I saw you already merged the commit in 5bfe16e. Can this pull request be closed?

@bep
Copy link
Member

bep commented Apr 8, 2016

Oops, I pushed that by mistake, but with those numbers it looks solid. I was a little bit afraid that people would shoot themselves in the feet and get a slow-down.

@bep bep closed this Apr 8, 2016
@digitalcraftsman digitalcraftsman deleted the tplfunc/findRE branch April 8, 2016 16:05
@digitalcraftsman
Copy link
Member Author

Should we add a note the docs that informs users about this potential bottleneck

@moorereason
Copy link
Contributor

People that are using regexp should know what they're getting into. Let's wait and see how it unfolds. If we start getting a bunch of people confused about why their builds are slow, we can consider updating the docs.

Although, I don't think it will be that much of a slowdown, anyway. The Go RE engine won't blow up in your face like all the other back-referencing engines can.

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants