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

Conversation

Projects
None yet
3 participants
@digitalcraftsman
Member

digitalcraftsman commented Apr 5, 2016

No description provided.

@digitalcraftsman digitalcraftsman changed the title from tpl: Add findRE templace func to tpl: Add findRE template func Apr 5, 2016

@bep

This comment has been minimized.

Show comment
Hide comment
@bep

bep Apr 5, 2016

Member

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

Member

bep commented Apr 5, 2016

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

@digitalcraftsman

This comment has been minimized.

Show comment
Hide comment
@digitalcraftsman

digitalcraftsman Apr 5, 2016

Member

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?

Member

digitalcraftsman commented Apr 5, 2016

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

This comment has been minimized.

Show comment
Hide comment
@bep

bep Apr 5, 2016

Member

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.

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

This comment has been minimized.

Show comment
Hide comment
@digitalcraftsman

digitalcraftsman Apr 5, 2016

Member

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.

Member

digitalcraftsman commented Apr 5, 2016

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

This comment has been minimized.

Show comment
Hide comment
@bep

bep Apr 5, 2016

Member

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

Member

bep commented Apr 5, 2016

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

@digitalcraftsman

This comment has been minimized.

Show comment
Hide comment
@digitalcraftsman

digitalcraftsman Apr 5, 2016

Member

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.

Member

digitalcraftsman commented Apr 5, 2016

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

This comment has been minimized.

Show comment
Hide comment
@moorereason

moorereason Apr 5, 2016

Contributor

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

Contributor

moorereason commented Apr 5, 2016

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

@digitalcraftsman

This comment has been minimized.

Show comment
Hide comment
@digitalcraftsman

digitalcraftsman Apr 5, 2016

Member

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 .

Member

digitalcraftsman commented Apr 5, 2016

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

This comment has been minimized.

Show comment
Hide comment
@bep

bep Apr 7, 2016

Member

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

Member

bep commented Apr 7, 2016

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

@digitalcraftsman

This comment has been minimized.

Show comment
Hide comment
@digitalcraftsman

digitalcraftsman Apr 8, 2016

Member

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?

Member

digitalcraftsman commented Apr 8, 2016

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

This comment has been minimized.

Show comment
Hide comment
@bep

bep Apr 8, 2016

Member

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.

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 digitalcraftsman:tplfunc/findRE branch Apr 8, 2016

@digitalcraftsman

This comment has been minimized.

Show comment
Hide comment
@digitalcraftsman

digitalcraftsman Apr 8, 2016

Member

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

Member

digitalcraftsman commented Apr 8, 2016

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

@moorereason

This comment has been minimized.

Show comment
Hide comment
@moorereason

moorereason Apr 8, 2016

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.

Contributor

moorereason commented Apr 8, 2016

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment