Skip to content

improve overall build speed by caching the result of the pages() method #201

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

Closed
wants to merge 2 commits into from

Conversation

chaudum
Copy link
Contributor

@chaudum chaudum commented Oct 19, 2015

additionally use os.walk instead of recursive fileList method

@krallin
Copy link
Collaborator

krallin commented Oct 19, 2015

Using os.walk is straightforward and probably something we should have done all along. I'd be happy to merge it separately.

Caching the result of site.pages() is a bit of a problem, though. Currently the contract is that if you make some changes to your pages during the build process (e.g. in a plugin), they'll be picked up next time pages() is evaluated.

I think this might break cactus serve (see the discussion here in #137). It might also break some plugins that depend on the current behavior, but I think it'd make sense to make a major release to address that.

Cheers,

@chaudum
Copy link
Contributor Author

chaudum commented Oct 19, 2015

Ok, that's a valid point I haven't thought about.
So I will make a separate PR for os.walk then and close this one.

@chaudum chaudum closed this Oct 19, 2015
@krallin
Copy link
Collaborator

krallin commented Oct 19, 2015

@chaudum I noticed you'd added some tracking of how long it takes to build the site with this change. Do you have any numbers you can share? Just curious to know how much effort we should be putting into this by looking at other real projects.

@chaudum
Copy link
Contributor Author

chaudum commented Oct 19, 2015

@krallin
In our website project (https://github.com/crate/crate-web) we have 207 pages which took ~15s to build before. After adding the cache it took around ~4.5s.

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.

2 participants