As discussed recently, we seem to spend a lot of time figuring out what
formats are exportable. The first of the attached patches introduces
caching of this information, in a very simple way. The second and third
use this to clean up a few things elsewhere: We can just sort this list
the first time we build it now, and not have to keep sorting it later.
The last just introduces a typedef, to make things more intelligible.

Comments welcome. What testing I've done shows it works. It'd be nice to
know if it actually shows a difference in how much time we're spending
in this method.

Richard


>From a6eaf99011504bf4626a992dcad353537116cf17 Mon Sep 17 00:00:00 2001
From: Richard Heck <rgh...@lyx.org>
Date: Wed, 19 Oct 2016 17:22:58 -0400
Subject: [PATCH 1/4] Simple cache for information on exportable formats, since
 we seem to access this information a lot.

---
 src/BufferParams.cpp | 30 ++++++++++++++++++++++++------
 src/BufferParams.h   |  4 ++--
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/src/BufferParams.cpp b/src/BufferParams.cpp
index 85a8cbb..4290430 100644
--- a/src/BufferParams.cpp
+++ b/src/BufferParams.cpp
@@ -355,11 +355,17 @@ public:
 	VSpace defskip;
 	PDFOptions pdfoptions;
 	LayoutFileIndex baseClass_;
+	/// Caching for exportableFormats, which seems to be slow.
+	std::vector<Format const *> exportableFormatList;
+	std::vector<Format const *> viewableFormatList;
+	bool isViewCacheValid;
+	bool isExportCacheValid;
 };
 
 
 BufferParams::Impl::Impl()
-	: defskip(VSpace::MEDSKIP), baseClass_(string(""))
+	: defskip(VSpace::MEDSKIP), baseClass_(string("")),
+	  isViewCacheValid(false), isExportCacheValid(false)
 {
 	// set initial author
 	// FIXME UNICODE
@@ -2251,6 +2257,8 @@ void BufferParams::setDocumentClass(DocumentClassConstPtr tc)
 {
 	// evil, but this function is evil
 	doc_class_ = const_pointer_cast<DocumentClass>(tc);
+	pimpl_->isExportCacheValid = false;
+	pimpl_->isViewCacheValid = false;
 }
 
 
@@ -2309,6 +2317,8 @@ void BufferParams::makeDocumentClass(bool const clone)
 	if (!baseClass())
 		return;
 
+	pimpl_->isExportCacheValid = false;
+	pimpl_->isViewCacheValid = false;
 	LayoutModuleList mods;
 	LayoutModuleList::iterator it = layout_modules_.begin();
 	LayoutModuleList::iterator en = layout_modules_.end();
@@ -2395,8 +2405,15 @@ bool BufferParams::isExportable(string const & format) const
 }
 
 
-vector<Format const *> BufferParams::exportableFormats(bool only_viewable) const
+vector<Format const *> const & BufferParams::exportableFormats(bool only_viewable) const
 {
+	vector<Format const *> & cached = only_viewable ?
+			pimpl_->viewableFormatList : pimpl_->exportableFormatList;
+	bool & valid = only_viewable ? 
+			pimpl_->isViewCacheValid : pimpl_->isExportCacheValid;
+	if (valid)
+		return cached;
+
 	vector<string> const backs = backends();
 	set<string> excludes;
 	if (useNonTeXFonts) {
@@ -2411,15 +2428,16 @@ vector<Format const *> BufferParams::exportableFormats(bool only_viewable) const
 			theConverters().getReachable(*it, only_viewable, false, excludes);
 		result.insert(result.end(), r.begin(), r.end());
 	}
-	return result;
+	cached = result;
+	valid = true;
+	return cached;
 }
 
 
 bool BufferParams::isExportableFormat(string const & format) const
 {
 	typedef vector<Format const *> Formats;
-	Formats formats;
-	formats = exportableFormats(true);
+	Formats const & formats = exportableFormats(true);
 	Formats::const_iterator fit = formats.begin();
 	Formats::const_iterator end = formats.end();
 	for (; fit != end ; ++fit) {
@@ -2516,7 +2534,7 @@ string BufferParams::getDefaultOutputFormat() const
 		return default_output_format;
 	if (isDocBook()
 	    || encoding().package() == Encoding::japanese) {
-		vector<Format const *> const formats = exportableFormats(true);
+		vector<Format const *> const & formats = exportableFormats(true);
 		if (formats.empty())
 			return string();
 		// return the first we find
diff --git a/src/BufferParams.h b/src/BufferParams.h
index 4efbe57..b18a7be 100644
--- a/src/BufferParams.h
+++ b/src/BufferParams.h
@@ -180,7 +180,7 @@ public:
 	///
 	bool isExportable(std::string const & format) const;
 	///
-	std::vector<Format const *> exportableFormats(bool only_viewable) const;
+	std::vector<const Format *> const & exportableFormats(bool only_viewable) const;
 	///
 	bool isExportableFormat(std::string const & format) const;
 	/// the backends appropriate for use with this document.
@@ -553,7 +553,7 @@ private:
 	 *  mathdots, stackrel, stmaryrd and undertilde.
 	 */
 	PackageMap use_packages;
-
+	
 	/** Use the Pimpl idiom to hide those member variables that would otherwise
 	 *  drag in other header files.
 	 */
-- 
2.7.4

>From dbe0c1377c0bba3cd32279045537afab58fed765 Mon Sep 17 00:00:00 2001
From: Richard Heck <rgh...@lyx.org>
Date: Wed, 19 Oct 2016 17:28:51 -0400
Subject: [PATCH 2/4] Since we're now caching this, we can sort it once, rather
 than lots of times.

---
 src/BufferParams.cpp              | 1 +
 src/frontends/qt4/GuiDocument.cpp | 4 ++--
 src/frontends/qt4/Menus.cpp       | 1 -
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/BufferParams.cpp b/src/BufferParams.cpp
index 4290430..0cebcc5 100644
--- a/src/BufferParams.cpp
+++ b/src/BufferParams.cpp
@@ -2428,6 +2428,7 @@ vector<Format const *> const & BufferParams::exportableFormats(bool only_viewabl
 			theConverters().getReachable(*it, only_viewable, false, excludes);
 		result.insert(result.end(), r.begin(), r.end());
 	}
+	sort(result.begin(), result.end(), Format::formatSorter);
 	cached = result;
 	valid = true;
 	return cached;
diff --git a/src/frontends/qt4/GuiDocument.cpp b/src/frontends/qt4/GuiDocument.cpp
index 09d9260..a3002f0 100644
--- a/src/frontends/qt4/GuiDocument.cpp
+++ b/src/frontends/qt4/GuiDocument.cpp
@@ -2561,8 +2561,8 @@ void GuiDocument::updateDefaultFormat()
 	outputModule->defaultFormatCO->clear();
 	outputModule->defaultFormatCO->addItem(qt_("Default"),
 				QVariant(QString("default")));
-	vector<Format const *> formats = param_copy.exportableFormats(true);
-	sort(formats.begin(), formats.end(), Format::formatSorter);
+	vector<Format const *> const & formats =
+				param_copy.exportableFormats(true);
 	for (Format const * f : formats)
 		outputModule->defaultFormatCO->addItem
 			(toqstr(translateIfPossible(f->prettyname())),
diff --git a/src/frontends/qt4/Menus.cpp b/src/frontends/qt4/Menus.cpp
index da1160a..16182e6 100644
--- a/src/frontends/qt4/Menus.cpp
+++ b/src/frontends/qt4/Menus.cpp
@@ -1051,7 +1051,6 @@ void MenuDefinition::expandFormats(MenuItem::Kind const kind, Buffer const * buf
 		LATTEST(false);
 		return;
 	}
-	sort(formats.begin(), formats.end(), Format::formatSorter);
 
 	bool const view_update = (kind == MenuItem::ViewFormats
 			|| kind == MenuItem::UpdateFormats);
-- 
2.7.4

>From b24fc4931a541373e7749f985eaedc997bac4447 Mon Sep 17 00:00:00 2001
From: Richard Heck <rgh...@lyx.org>
Date: Wed, 19 Oct 2016 17:30:48 -0400
Subject: [PATCH 3/4] There is no need now to cache format information in the
 SendTo dialog.

---
 src/frontends/qt4/GuiSendto.cpp | 14 ++++++++------
 src/frontends/qt4/GuiSendto.h   |  2 --
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/frontends/qt4/GuiSendto.cpp b/src/frontends/qt4/GuiSendto.cpp
index 13ba03d..a3c125e 100644
--- a/src/frontends/qt4/GuiSendto.cpp
+++ b/src/frontends/qt4/GuiSendto.cpp
@@ -71,17 +71,17 @@ void GuiSendTo::changed_adaptor()
 
 void GuiSendTo::updateContents()
 {
-	all_formats_ = buffer().params().exportableFormats(false);
-	sort(all_formats_.begin(), all_formats_.end(), Format::formatSorter);
+	vector<Format const *> const & all_formats = 
+	    buffer().params().exportableFormats(false);
 	// Save the current selection if any
 	Format const * current_format = nullptr;
 	int const line = formatLW->currentRow();
-	if (line >= 0 && static_cast<unsigned int>(line) < all_formats_.size()
+	if (line >= 0 && static_cast<unsigned int>(line) < all_formats.size()
 	    && formatLW->selectedItems().size() > 0)
-		current_format = all_formats_[line];
+		current_format = all_formats[line];
 	// Reset the list widget
 	formatLW->clear();
-	for (Format const * f : all_formats_) {
+	for (Format const * f : all_formats) {
 		formatLW->addItem(toqstr(translateIfPossible(f->prettyname())));
 		// Restore the selection
 		if (current_format && f->prettyname() == current_format->prettyname())
@@ -101,7 +101,9 @@ void GuiSendTo::applyView()
 	if (line < 0 || line > formatLW->count())
 		return;
 
-	format_ = all_formats_[line];
+	vector<Format const *> const & all_formats = 
+	    buffer().params().exportableFormats(false);
+	format_ = all_formats[line];
 	command_ = command;
 }
 
diff --git a/src/frontends/qt4/GuiSendto.h b/src/frontends/qt4/GuiSendto.h
index 38f0f53..ba0a245 100644
--- a/src/frontends/qt4/GuiSendto.h
+++ b/src/frontends/qt4/GuiSendto.h
@@ -47,8 +47,6 @@ private:
 	void updateContents();
 
 	///
-	std::vector<Format const *> all_formats_;
-	///
 	bool initialiseParams(std::string const & data);
 	///
 	void paramsToDialog(Format const * format, QString const & command);
-- 
2.7.4

>From e3758a7209100efe939adb7d22391797e60261db Mon Sep 17 00:00:00 2001
From: Richard Heck <rgh...@lyx.org>
Date: Wed, 19 Oct 2016 18:11:58 -0400
Subject: [PATCH 4/4] Use a typedef for vector<Format const *>, which is what
 gets used for lists of exportable and importable formats.

---
 src/BufferParams.cpp              | 24 +++++++++++-------------
 src/BufferParams.h                |  2 +-
 src/Converter.cpp                 | 30 ++++++++++++++----------------
 src/Converter.h                   | 20 +++++++++++---------
 src/frontends/qt4/GuiDocument.cpp |  3 ++-
 src/frontends/qt4/GuiSendto.cpp   |  5 +++--
 src/frontends/qt4/Menus.cpp       |  3 +--
 src/frontends/qt4/Toolbars.cpp    |  2 +-
 8 files changed, 44 insertions(+), 45 deletions(-)

diff --git a/src/BufferParams.cpp b/src/BufferParams.cpp
index 0cebcc5..a515233c 100644
--- a/src/BufferParams.cpp
+++ b/src/BufferParams.cpp
@@ -355,9 +355,8 @@ public:
 	VSpace defskip;
 	PDFOptions pdfoptions;
 	LayoutFileIndex baseClass_;
-	/// Caching for exportableFormats, which seems to be slow.
-	std::vector<Format const *> exportableFormatList;
-	std::vector<Format const *> viewableFormatList;
+	FormatList exportableFormatList;
+	FormatList viewableFormatList;
 	bool isViewCacheValid;
 	bool isExportCacheValid;
 };
@@ -2405,9 +2404,9 @@ bool BufferParams::isExportable(string const & format) const
 }
 
 
-vector<Format const *> const & BufferParams::exportableFormats(bool only_viewable) const
+FormatList const & BufferParams::exportableFormats(bool only_viewable) const
 {
-	vector<Format const *> & cached = only_viewable ?
+	FormatList & cached = only_viewable ?
 			pimpl_->viewableFormatList : pimpl_->exportableFormatList;
 	bool & valid = only_viewable ? 
 			pimpl_->isViewCacheValid : pimpl_->isExportCacheValid;
@@ -2420,12 +2419,12 @@ vector<Format const *> const & BufferParams::exportableFormats(bool only_viewabl
 		excludes.insert("latex");
 		excludes.insert("pdflatex");
 	}
-	vector<Format const *> result =
+	FormatList result =
 		theConverters().getReachable(backs[0], only_viewable, true, excludes);
 	for (vector<string>::const_iterator it = backs.begin() + 1;
 	     it != backs.end(); ++it) {
-		vector<Format const *>  r =
-			theConverters().getReachable(*it, only_viewable, false, excludes);
+		FormatList r = theConverters().getReachable(*it, only_viewable, 
+				false, excludes);
 		result.insert(result.end(), r.begin(), r.end());
 	}
 	sort(result.begin(), result.end(), Format::formatSorter);
@@ -2437,10 +2436,9 @@ vector<Format const *> const & BufferParams::exportableFormats(bool only_viewabl
 
 bool BufferParams::isExportableFormat(string const & format) const
 {
-	typedef vector<Format const *> Formats;
-	Formats const & formats = exportableFormats(true);
-	Formats::const_iterator fit = formats.begin();
-	Formats::const_iterator end = formats.end();
+	FormatList const & formats = exportableFormats(true);
+	FormatList::const_iterator fit = formats.begin();
+	FormatList::const_iterator end = formats.end();
 	for (; fit != end ; ++fit) {
 		if ((*fit)->name() == format)
 			return true;
@@ -2535,7 +2533,7 @@ string BufferParams::getDefaultOutputFormat() const
 		return default_output_format;
 	if (isDocBook()
 	    || encoding().package() == Encoding::japanese) {
-		vector<Format const *> const & formats = exportableFormats(true);
+		FormatList const & formats = exportableFormats(true);
 		if (formats.empty())
 			return string();
 		// return the first we find
diff --git a/src/BufferParams.h b/src/BufferParams.h
index b18a7be..f584a29 100644
--- a/src/BufferParams.h
+++ b/src/BufferParams.h
@@ -553,7 +553,7 @@ private:
 	 *  mathdots, stackrel, stmaryrd and undertilde.
 	 */
 	PackageMap use_packages;
-	
+
 	/** Use the Pimpl idiom to hide those member variables that would otherwise
 	 *  drag in other header files.
 	 */
diff --git a/src/Converter.cpp b/src/Converter.cpp
index 465e256..58e486e 100644
--- a/src/Converter.cpp
+++ b/src/Converter.cpp
@@ -717,14 +717,13 @@ void Converters::buildGraph()
 }
 
 
-vector<Format const *> const
-Converters::intToFormat(vector<int> const & input)
+FormatList const Converters::intToFormat(vector<int> const & input)
 {
-	vector<Format const *> result(input.size());
+	FormatList result(input.size());
 
 	vector<int>::const_iterator it = input.begin();
 	vector<int>::const_iterator const end = input.end();
-	vector<Format const *>::iterator rit = result.begin();
+	FormatList::iterator rit = result.begin();
 	for ( ; it != end; ++it, ++rit) {
 		*rit = &formats.get(*it);
 	}
@@ -732,8 +731,8 @@ Converters::intToFormat(vector<int> const & input)
 }
 
 
-vector<Format const *> const
-Converters::getReachableTo(string const & target, bool const clear_visited)
+FormatList const Converters::getReachableTo(string const & target, 
+		bool const clear_visited)
 {
 	vector<int> const & reachablesto =
 		G_.getReachableTo(formats.getNumber(target), clear_visited);
@@ -742,9 +741,9 @@ Converters::getReachableTo(string const & target, bool const clear_visited)
 }
 
 
-vector<Format const *> const
-Converters::getReachable(string const & from, bool const only_viewable,
-			 bool const clear_visited, set<string> const & excludes)
+FormatList const Converters::getReachable(string const & from, 
+		bool const only_viewable, bool const clear_visited, 
+		set<string> const & excludes)
 {
 	set<int> excluded_numbers;
 
@@ -777,29 +776,28 @@ Graph::EdgePath Converters::getPath(string const & from, string const & to)
 }
 
 
-vector<Format const *> Converters::importableFormats()
+FormatList Converters::importableFormats()
 {
 	vector<string> l = loaders();
-	vector<Format const *> result = getReachableTo(l[0], true);
+	FormatList result = getReachableTo(l[0], true);
 	vector<string>::const_iterator it = l.begin() + 1;
 	vector<string>::const_iterator en = l.end();
 	for (; it != en; ++it) {
-		vector<Format const *> r = getReachableTo(*it, false);
+		FormatList r = getReachableTo(*it, false);
 		result.insert(result.end(), r.begin(), r.end());
 	}
 	return result;
 }
 
 
-vector<Format const *> Converters::exportableFormats(bool only_viewable)
+FormatList Converters::exportableFormats(bool only_viewable)
 {
 	vector<string> s = savers();
-	vector<Format const *> result = getReachable(s[0], only_viewable, true);
+	FormatList result = getReachable(s[0], only_viewable, true);
 	vector<string>::const_iterator it = s.begin() + 1;
 	vector<string>::const_iterator en = s.end();
 	for (; it != en; ++it) {
-		vector<Format const *> r =
-			getReachable(*it, only_viewable, false);
+		 FormatList r = getReachable(*it, only_viewable, false);
 		result.insert(result.end(), r.begin(), r.end());
 	}
 	return result;
diff --git a/src/Converter.h b/src/Converter.h
index 082b2d3..c9c4423 100644
--- a/src/Converter.h
+++ b/src/Converter.h
@@ -30,6 +30,8 @@ class ErrorList;
 class Format;
 class Formats;
 
+typedef std::vector<Format const *> FormatList;
+
 
 ///
 class Converter {
@@ -131,16 +133,16 @@ public:
 	//
 	void erase(std::string const & from, std::string const & to);
 	///
-	std::vector<Format const *> const
-	getReachableTo(std::string const & target, bool clear_visited);
+	FormatList const 
+		getReachableTo(std::string const & target, bool clear_visited);
 	///
-	std::vector<Format const *> const
-	getReachable(std::string const & from, bool only_viewable,
-	       bool clear_visited,
-	       std::set<std::string> const & excludes = std::set<std::string>());
+	FormatList const
+		getReachable(std::string const & from, bool only_viewable,
+		    bool clear_visited,
+		    std::set<std::string> const & excludes = std::set<std::string>());
 
-	std::vector<Format const *> importableFormats();
-	std::vector<Format const *> exportableFormats(bool only_viewable);
+	FormatList importableFormats();
+	FormatList exportableFormats(bool only_viewable);
 
 	std::vector<std::string> loaders() const;
 	std::vector<std::string> savers() const;
@@ -181,7 +183,7 @@ public:
 	void buildGraph();
 private:
 	///
-	std::vector<Format const *> const
+	FormatList const
 	intToFormat(std::vector<int> const & input);
 	///
 	bool scanLog(Buffer const & buffer, std::string const & command,
diff --git a/src/frontends/qt4/GuiDocument.cpp b/src/frontends/qt4/GuiDocument.cpp
index a3002f0..2a40873 100644
--- a/src/frontends/qt4/GuiDocument.cpp
+++ b/src/frontends/qt4/GuiDocument.cpp
@@ -31,6 +31,7 @@
 #include "BufferView.h"
 #include "Color.h"
 #include "ColorCache.h"
+#include "Converter.h"
 #include "Cursor.h"
 #include "Encoding.h"
 #include "FloatPlacement.h"
@@ -2561,7 +2562,7 @@ void GuiDocument::updateDefaultFormat()
 	outputModule->defaultFormatCO->clear();
 	outputModule->defaultFormatCO->addItem(qt_("Default"),
 				QVariant(QString("default")));
-	vector<Format const *> const & formats =
+	FormatList const & formats =
 				param_copy.exportableFormats(true);
 	for (Format const * f : formats)
 		outputModule->defaultFormatCO->addItem
diff --git a/src/frontends/qt4/GuiSendto.cpp b/src/frontends/qt4/GuiSendto.cpp
index a3c125e..92814f0 100644
--- a/src/frontends/qt4/GuiSendto.cpp
+++ b/src/frontends/qt4/GuiSendto.cpp
@@ -16,6 +16,7 @@
 
 #include "Buffer.h"
 #include "BufferParams.h"
+#include "Converter.h"
 #include "Format.h"
 #include "FuncRequest.h"
 
@@ -71,7 +72,7 @@ void GuiSendTo::changed_adaptor()
 
 void GuiSendTo::updateContents()
 {
-	vector<Format const *> const & all_formats = 
+	FormatList const & all_formats = 
 	    buffer().params().exportableFormats(false);
 	// Save the current selection if any
 	Format const * current_format = nullptr;
@@ -101,7 +102,7 @@ void GuiSendTo::applyView()
 	if (line < 0 || line > formatLW->count())
 		return;
 
-	vector<Format const *> const & all_formats = 
+	FormatList const & all_formats = 
 	    buffer().params().exportableFormats(false);
 	format_ = all_formats[line];
 	command_ = command;
diff --git a/src/frontends/qt4/Menus.cpp b/src/frontends/qt4/Menus.cpp
index 16182e6..53a8455 100644
--- a/src/frontends/qt4/Menus.cpp
+++ b/src/frontends/qt4/Menus.cpp
@@ -1026,8 +1026,7 @@ void MenuDefinition::expandFormats(MenuItem::Kind const kind, Buffer const * buf
 	if (!buf && kind != MenuItem::ImportFormats)
 		return;
 
-	typedef vector<Format const *> Formats;
-	Formats formats;
+	FormatList formats;
 	FuncCode action = LFUN_NOACTION;
 
 	switch (kind) {
diff --git a/src/frontends/qt4/Toolbars.cpp b/src/frontends/qt4/Toolbars.cpp
index b2a9c37..2c0d2ab 100644
--- a/src/frontends/qt4/Toolbars.cpp
+++ b/src/frontends/qt4/Toolbars.cpp
@@ -194,7 +194,7 @@ ToolbarInfo & ToolbarInfo::read(Lexer & lex)
 		case TO_IMPORTFORMATS:
 		case TO_UPDATEFORMATS:
 		case TO_VIEWFORMATS: {
-			vector<Format const *> formats = (code == TO_IMPORTFORMATS) ?
+			FormatList formats = (code == TO_IMPORTFORMATS) ?
 				theConverters().importableFormats() :
 				theConverters().exportableFormats(true);
 			sort(formats.begin(), formats.end());
-- 
2.7.4

Reply via email to