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