From 14560f224ea02df4df80db356a497720a08e0abb Mon Sep 17 00:00:00 2001 From: Vincent Prouillet Date: Thu, 20 Sep 2018 22:19:52 +0200 Subject: [PATCH] No clone when rendering rss feed --- components/content/src/lib.rs | 2 +- components/content/src/sorting.rs | 15 +++++++++++++++ components/rebuild/src/lib.rs | 2 +- components/site/benches/site.rs | 8 ++++---- components/site/src/lib.rs | 23 ++++++++++------------- 5 files changed, 31 insertions(+), 19 deletions(-) diff --git a/components/content/src/lib.rs b/components/content/src/lib.rs index 11bfcc85..1682ad0e 100644 --- a/components/content/src/lib.rs +++ b/components/content/src/lib.rs @@ -26,4 +26,4 @@ mod sorting; pub use file_info::FileInfo; pub use page::Page; pub use section::Section; -pub use sorting::{sort_pages, populate_siblings}; +pub use sorting::{sort_pages, populate_siblings, sort_pages_by_date}; diff --git a/components/content/src/sorting.rs b/components/content/src/sorting.rs index bf1f1751..df3c3e82 100644 --- a/components/content/src/sorting.rs +++ b/components/content/src/sorting.rs @@ -5,6 +5,21 @@ use rayon::prelude::*; use page::Page; use front_matter::SortBy; + +/// The comparison function of sorting pages by day +/// Used by the RSS rendering +/// To remove if `sort_pages` is changed to work on borrowed values +/// This cannot be used in `sort_pages` currently as it takes &&Page instead of &Page +pub fn sort_pages_by_date(a: &&Page, b: &&Page) -> Ordering { + let ord = b.meta.date().unwrap().cmp(&a.meta.date().unwrap()); + if ord == Ordering::Equal { + a.permalink.cmp(&b.permalink) + } else { + ord + } +} + + /// Sort pages by the given criteria /// /// Any pages that doesn't have a required field when the sorting method is other than none diff --git a/components/rebuild/src/lib.rs b/components/rebuild/src/lib.rs index fb47eaec..07652a8c 100644 --- a/components/rebuild/src/lib.rs +++ b/components/rebuild/src/lib.rs @@ -309,7 +309,7 @@ pub fn after_template_change(site: &mut Site, path: &Path) -> Result<()> { match filename { "sitemap.xml" => site.render_sitemap(), - "rss.xml" => site.render_rss_feed(None, None), + "rss.xml" => site.render_rss_feed(site.pages.values().collect(), None), "robots.txt" => site.render_robots(), "single.html" | "list.html" => site.render_taxonomies(), "page.html" => { diff --git a/components/site/benches/site.rs b/components/site/benches/site.rs index 79e2ec63..e0eb4595 100644 --- a/components/site/benches/site.rs +++ b/components/site/benches/site.rs @@ -22,7 +22,7 @@ fn setup_site(name: &str) -> Site { #[bench] fn bench_render_aliases(b: &mut test::Bencher) { - let mut site = setup_site("small-blog"); + let mut site = setup_site("big-blog"); let tmp_dir = tempdir().expect("create temp dir"); let public = &tmp_dir.path().join("public"); site.set_output_path(&public); @@ -31,7 +31,7 @@ fn bench_render_aliases(b: &mut test::Bencher) { #[bench] fn bench_render_sitemap(b: &mut test::Bencher) { - let mut site = setup_site("small-blog"); + let mut site = setup_site("big-blog"); let tmp_dir = tempdir().expect("create temp dir"); let public = &tmp_dir.path().join("public"); site.set_output_path(&public); @@ -40,11 +40,11 @@ fn bench_render_sitemap(b: &mut test::Bencher) { #[bench] fn bench_render_rss_feed(b: &mut test::Bencher) { - let mut site = setup_site("small-blog"); + let mut site = setup_site("big-blog"); let tmp_dir = tempdir().expect("create temp dir"); let public = &tmp_dir.path().join("public"); site.set_output_path(&public); - b.iter(|| site.render_rss_feed(None, None).unwrap()); + b.iter(|| site.render_rss_feed(site.pages.values().collect(), None).unwrap()); } #[bench] diff --git a/components/site/src/lib.rs b/components/site/src/lib.rs index 196e7ed3..efb556d1 100644 --- a/components/site/src/lib.rs +++ b/components/site/src/lib.rs @@ -36,9 +36,9 @@ use config::{Config, get_config}; use utils::fs::{create_file, copy_directory, create_directory, ensure_directory_exists}; use utils::templates::{render_template, rewrite_theme_paths}; use utils::net::get_available_port; -use content::{Page, Section, populate_siblings, sort_pages}; +use content::{Page, Section, populate_siblings, sort_pages, sort_pages_by_date}; use templates::{GUTENBERG_TERA, global_fns, render_redirect_template}; -use front_matter::{SortBy, InsertAnchor}; +use front_matter::{InsertAnchor}; use taxonomies::{Taxonomy, find_taxonomies}; use pagination::Paginator; @@ -515,7 +515,7 @@ impl Site { self.render_orphan_pages()?; self.render_sitemap()?; if self.config.generate_rss { - self.render_rss_feed(None, None)?; + self.render_rss_feed(self.pages.values().collect(), None)?; } self.render_404()?; self.render_robots()?; @@ -695,9 +695,8 @@ impl Site { .par_iter() .map(|item| { if taxonomy.kind.rss { - // TODO: can we get rid of `clone()`? self.render_rss_feed( - Some(item.pages.clone()), + item.pages.iter().map(|p| p).collect(), Some(&PathBuf::from(format!("{}/{}", taxonomy.kind.name, item.slug))), )?; } @@ -769,27 +768,25 @@ impl Site { /// Renders a RSS feed for the given path and at the given path /// If both arguments are `None`, it will render only the RSS feed for the whole /// site at the root folder. - pub fn render_rss_feed(&self, all_pages: Option>, base_path: Option<&PathBuf>) -> Result<()> { + pub fn render_rss_feed(&self, all_pages: Vec<&Page>, base_path: Option<&PathBuf>) -> Result<()> { ensure_directory_exists(&self.output_path)?; let mut context = Context::new(); - let pages = all_pages - // TODO: avoid that cloned(). - // It requires having `sort_pages` take references of Page - .unwrap_or_else(|| self.pages.values().cloned().collect::>()) + let mut pages = all_pages .into_iter() .filter(|p| p.meta.date.is_some() && !p.is_draft()) .collect::>(); + pages.par_sort_unstable_by(sort_pages_by_date); + // Don't generate a RSS feed if none of the pages has a date if pages.is_empty() { return Ok(()); } - let (sorted_pages, _) = sort_pages(pages, SortBy::Date); - context.insert("last_build_date", &sorted_pages[0].meta.date.clone().map(|d| d.to_string())); + context.insert("last_build_date", &pages[0].meta.date.clone().map(|d| d.to_string())); // limit to the last n elements - context.insert("pages", &sorted_pages.iter().take(self.config.rss_limit).collect::>()); + context.insert("pages", &pages.iter().take(self.config.rss_limit).collect::>()); context.insert("config", &self.config); let rss_feed_url = if let Some(ref base) = base_path {