commit 7c614cc6b723bbdd5a65a5eb6dab8c400b2ec017
Author: Richard Kimberly Heck <[email protected]>
Date: Sun Sep 2 00:10:01 2018 -0400
Try yet again try to fix bug #9158.
The problem with the previous attempt was that, every time through
updateBuffer, we looked up the file location using kpsewhich, which
took too long on Windows. The new solution is to cache that info, and
to look it up only when we need it.
Previously, this info would have been re-read whenever we parsed the
bibfiles. So we re-read it now whenever the bibinfo cache is invalid,
which is less often, but should be good enough. We can add more such
re-reads if need be.
(cherry picked from commit 601449269939150bb7452698809e59c00a98a20f)
---
src/Buffer.cpp | 132 ++++++++++++++++++++++++++++++++-----------
src/Buffer.h | 11 ++-
src/BufferView.cpp | 5 +-
src/insets/InsetBibtex.cpp | 56 ++++---------------
src/insets/InsetBibtex.h | 7 +--
src/insets/InsetInclude.cpp | 7 --
src/support/FileNameList.h | 10 ---
7 files changed, 119 insertions(+), 109 deletions(-)
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index 71b26f9..4ea890a 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -279,7 +279,7 @@ public:
/// A cache for the bibfiles (including bibfiles of loaded child
/// documents), needed for appropriate update of natbib labels.
- mutable support::FileNamePairList bibfiles_cache_;
+ mutable docstring_list bibfiles_cache_;
// FIXME The caching mechanism could be improved. At present, we have a
// cache for each Buffer, that caches all the bibliography info for that
@@ -344,7 +344,6 @@ public:
LYXERR0("Warning: a buffer should not have two
parents!");
parent_buffer = pb;
if (!cloned_buffer_ && parent_buffer) {
- parent_buffer->invalidateBibfileCache();
parent_buffer->invalidateBibinfoCache();
}
}
@@ -1904,7 +1903,7 @@ void Buffer::writeLaTeXSource(otexstream & os,
// Biblatex bibliographies are loaded here
if (params().useBiblatex()) {
vector<docstring> const bibfiles =
- prepareBibFilePaths(runparams,
getBibfilesCache(), true);
+ prepareBibFilePaths(runparams, getBibfiles(), true);
for (docstring const & file: bibfiles)
os << "\\addbibresource{" << file << "}\n";
}
@@ -2335,7 +2334,7 @@ void Buffer::updateBibfilesCache(UpdateScope scope) const
for (InsetIterator it = inset_iterator_begin(inset()); it; ++it) {
if (it->lyxCode() == BIBTEX_CODE) {
InsetBibtex const & inset = static_cast<InsetBibtex
const &>(*it);
- support::FileNamePairList const bibfiles =
inset.getBibFiles();
+ docstring_list const bibfiles = inset.getBibFiles();
d->bibfiles_cache_.insert(d->bibfiles_cache_.end(),
bibfiles.begin(),
bibfiles.end());
@@ -2344,8 +2343,8 @@ void Buffer::updateBibfilesCache(UpdateScope scope) const
Buffer const * const incbuf = inset.getChildBuffer();
if (!incbuf)
continue;
- support::FileNamePairList const & bibfiles =
-
incbuf->getBibfilesCache(UpdateChildOnly);
+ docstring_list const & bibfiles =
+ incbuf->getBibfiles(UpdateChildOnly);
if (!bibfiles.empty()) {
d->bibfiles_cache_.insert(d->bibfiles_cache_.end(),
bibfiles.begin(),
@@ -2370,25 +2369,13 @@ void Buffer::invalidateBibinfoCache() const
}
-void Buffer::invalidateBibfileCache() const
-{
- d->bibfile_cache_valid_ = false;
- d->bibinfo_cache_valid_ = false;
- d->cite_labels_valid_ = false;
- // also invalidate the cache for the parent buffer
- Buffer const * const pbuf = d->parent();
- if (pbuf)
- pbuf->invalidateBibfileCache();
-}
-
-
-support::FileNamePairList const & Buffer::getBibfilesCache(UpdateScope scope)
const
+docstring_list const & Buffer::getBibfiles(UpdateScope scope) const
{
// FIXME This is probably unnecessary, given where we call this.
// If this is a child document, use the master's cache instead.
Buffer const * const pbuf = masterBuffer();
if (pbuf != this && scope != UpdateChildOnly)
- return pbuf->getBibfilesCache();
+ return pbuf->getBibfiles();
if (!d->bibfile_cache_valid_)
this->updateBibfilesCache(scope);
@@ -2412,6 +2399,49 @@ BiblioInfo const & Buffer::bibInfo() const
}
+void Buffer::registerBibfiles(const docstring_list & bf) const
+{
+ // We register the bib files in the master buffer,
+ // if there is one, but also in every single buffer,
+ // in case a child is compiled alone.
+ Buffer const * const tmp = masterBuffer();
+ if (tmp != this)
+ tmp->registerBibfiles(bf);
+
+ for (auto const & p : bf) {
+ docstring_list::const_iterator temp =
+ find(d->bibfiles_cache_.begin(),
d->bibfiles_cache_.end(), p);
+ if (temp == d->bibfiles_cache_.end())
+ d->bibfiles_cache_.push_back(p);
+ }
+}
+
+
+static map<docstring, FileName> bibfileCache;
+
+FileName Buffer::getBibfilePath(docstring const & bibid) const
+{
+ map<docstring, FileName>::const_iterator it =
+ bibfileCache.find(bibid);
+ if (it != bibfileCache.end()) {
+ // i.e., bibfileCache[bibid]
+ return it->second;
+ }
+
+ LYXERR(Debug::FILES, "Reading file location for " << bibid);
+ string texfile = changeExtension(to_utf8(bibid), "bib");
+ // note that, if the filename can be found directly from the path,
+ // findtexfile will just return a FileName object for that path.
+ FileName file(findtexfile(texfile, "bib"));
+ if (file.empty())
+ file = FileName(makeAbsPath(texfile, filePath()));
+ LYXERR(Debug::FILES, "Found at: " << file);
+
+ bibfileCache[bibid] = file;
+ return bibfileCache[bibid];
+}
+
+
void Buffer::checkIfBibInfoCacheIsValid() const
{
// use the master's cache
@@ -2422,11 +2452,9 @@ void Buffer::checkIfBibInfoCacheIsValid() const
}
// compare the cached timestamps with the actual ones.
- FileNamePairList const & bibfiles_cache = getBibfilesCache();
- FileNamePairList::const_iterator ei = bibfiles_cache.begin();
- FileNamePairList::const_iterator en = bibfiles_cache.end();
- for (; ei != en; ++ ei) {
- FileName const fn = ei->second;
+ docstring_list const & bibfiles_cache = getBibfiles();
+ for (auto const & bf : bibfiles_cache) {
+ FileName const fn = getBibfilePath(bf);
time_t lastw = fn.lastModified();
time_t prevw = d->bibfile_status_[fn];
if (lastw != prevw) {
@@ -2451,6 +2479,11 @@ void Buffer::reloadBibInfoCache() const
if (d->bibinfo_cache_valid_)
return;
+ // re-read file locations when this info changes
+ // FIXME Is this sufficient? Or should we also force that
+ // in some other cases? If so, then it is easy enough to
+ // add the following line in some other places.
+ bibfileCache.clear();
d->bibinfo_.clear();
FileNameList checkedFiles;
collectBibKeys(checkedFiles);
@@ -3173,7 +3206,7 @@ string const Buffer::prepareFileNameForLaTeX(string const
& name,
vector<docstring> const Buffer::prepareBibFilePaths(OutputParams const &
runparams,
- FileNamePairList const
bibfilelist,
+ docstring_list const &
bibfilelist,
bool const add_extension) const
{
// If we are processing the LaTeX file in a temp directory then
@@ -3194,10 +3227,8 @@ vector<docstring> const
Buffer::prepareBibFilePaths(OutputParams const & runpara
// check for spaces in paths
bool found_space = false;
- FileNamePairList::const_iterator it = bibfilelist.begin();
- FileNamePairList::const_iterator en = bibfilelist.end();
- for (; it != en; ++it) {
- string utf8input = to_utf8(it->first);
+ for (auto const & bit : bibfilelist) {
+ string utf8input = to_utf8(bit);
string database =
prepareFileNameForLaTeX(utf8input, ".bib",
runparams.nice);
FileName try_in_file =
@@ -3206,7 +3237,7 @@ vector<docstring> const
Buffer::prepareBibFilePaths(OutputParams const & runpara
// If the file has not been found, try with the real file name
// (it might come from a child in a sub-directory)
if (!not_from_texmf) {
- try_in_file = it->second;
+ try_in_file = getBibfilePath(bit);
if (try_in_file.isReadableFile()) {
// Check if the file is in texmf
FileName
kpsefile(findtexfile(changeExtension(utf8input, "bib"), "bib", true));
@@ -4761,8 +4792,10 @@ void Buffer::updateBuffer(UpdateScope scope, UpdateType
utype) const
Buffer const * const master = masterBuffer();
DocumentClass const & textclass = master->params().documentClass();
- // do this only if we are the top-level Buffer
- if (master == this) {
+ docstring_list old_bibfiles;
+ // Do this only if we are the top-level Buffer. We also need to account
+ // for the case of a previewed child with ignored parent here.
+ if (master == this && !d->ignore_parent) {
textclass.counters().reset(from_ascii("bibitem"));
reloadBibInfoCache();
}
@@ -4817,7 +4850,38 @@ void Buffer::updateBuffer(UpdateScope scope, UpdateType
utype) const
// in InsetInclude::addToToc.
return;
- d->bibinfo_cache_valid_ = true;
+ // if the bibfiles changed, the cache of bibinfo is invalid
+ docstring_list new_bibfiles = d->bibfiles_cache_;
+ // this is a trick to determine whether the two vectors have
+ // the same elements.
+ sort(new_bibfiles.begin(), new_bibfiles.end());
+ sort(old_bibfiles.begin(), old_bibfiles.end());
+ if (old_bibfiles != new_bibfiles) {
+ LYXERR(Debug::FILES, "Reloading bibinfo cache.");
+ invalidateBibinfoCache();
+ reloadBibInfoCache();
+ // We relied upon the bibinfo cache when recalculating labels.
But that
+ // cache was invalid, although we didn't find that out until
now. So we
+ // have to do it all again.
+ // That said, the only thing we really need to do is update the
citation
+ // labels. Nothing else will have changed. So we could create a
new
+ // UpdateType that would signal that fact, if we needed to do
so.
+ parit = cbuf.par_iterator_begin();
+ // we will be re-doing the counters and references and such.
+ textclass.counters().reset();
+ clearReferenceCache();
+ // we should not need to do this again?
+ // updateMacros();
+ setChangesPresent(false);
+ updateBuffer(parit, utype);
+ // this will already have been done by reloadBibInfoCache();
+ // d->bibinfo_cache_valid_ = true;
+ }
+ else {
+ LYXERR(Debug::FILES, "Bibfiles unchanged.");
+ // this is also set to true on the other path, by
reloadBibInfoCache.
+ d->bibinfo_cache_valid_ = true;
+ }
d->cite_labels_valid_ = true;
/// FIXME: Perf
cbuf.tocBackend().update(true, utype);
diff --git a/src/Buffer.h b/src/Buffer.h
index a68ad04..b666b66 100644
--- a/src/Buffer.h
+++ b/src/Buffer.h
@@ -70,7 +70,6 @@ class WorkAreaManager;
namespace support {
class DocFileName;
class FileName;
-class FileNamePairList;
} // namespace support
namespace graphics {
@@ -416,7 +415,7 @@ public:
* output in the respective BibTeX/Biblatex macro
*/
std::vector<docstring> const prepareBibFilePaths(OutputParams const &,
- support::FileNamePairList const bibfilelist,
+ const docstring_list & bibfilelist,
bool const extension = true) const;
/** Returns the path where a local layout file lives.
@@ -779,6 +778,10 @@ public:
void setChangesPresent(bool) const;
bool areChangesPresent() const;
void updateChangesPresent() const;
+ ///
+ void registerBibfiles(docstring_list const & bf) const;
+ ///
+ support::FileName getBibfilePath(docstring const & bibid) const;
private:
friend class MarkAsExporting;
@@ -800,8 +803,8 @@ private:
void updateBibfilesCache(UpdateScope scope = UpdateMaster) const;
/// Return the list with all bibfiles in use (including bibfiles
/// of loaded child documents).
- support::FileNamePairList const &
- getBibfilesCache(UpdateScope scope = UpdateMaster) const;
+ docstring_list const &
+ getBibfiles(UpdateScope scope = UpdateMaster) const;
///
void collectChildren(ListOfBuffers & children, bool grand_children)
const;
diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index 17dedce..d2f324d 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -1664,7 +1664,6 @@ void BufferView::dispatch(FuncRequest const & cmd,
DispatchResult & dr)
BIBTEX_CODE);
if (inset) {
if (inset->addDatabase(cmd.argument())) {
- buffer_.invalidateBibfileCache();
dr.forceBufferUpdate();
}
}
@@ -1677,10 +1676,8 @@ void BufferView::dispatch(FuncRequest const & cmd,
DispatchResult & dr)
InsetBibtex * inset = getInsetByCode<InsetBibtex>(tmpcur,
BIBTEX_CODE);
if (inset) {
- if (inset->delDatabase(cmd.argument())) {
- buffer_.invalidateBibfileCache();
+ if (inset->delDatabase(cmd.argument()))
dr.forceBufferUpdate();
- }
}
break;
}
diff --git a/src/insets/InsetBibtex.cpp b/src/insets/InsetBibtex.cpp
index 2d16496..7a04593 100644
--- a/src/insets/InsetBibtex.cpp
+++ b/src/insets/InsetBibtex.cpp
@@ -37,6 +37,7 @@
#include "support/convert.h"
#include "support/debug.h"
#include "support/docstream.h"
+#include "support/docstring_list.h"
#include "support/ExceptionMessage.h"
#include "support/FileNameList.h"
#include "support/filetools.h"
@@ -60,7 +61,6 @@ namespace os = support::os;
InsetBibtex::InsetBibtex(Buffer * buf, InsetCommandParams const & p)
: InsetCommand(buf, p)
{
- buffer().invalidateBibfileCache();
buffer().removeBiblioTempFiles();
}
@@ -71,7 +71,6 @@ InsetBibtex::~InsetBibtex()
/* We do not use buffer() because Coverity believes that this
* may throw an exception. Actually this code path is not
* taken when buffer_ == 0 */
- buffer_-> invalidateBibfileCache();
buffer_->removeBiblioTempFiles();
}
}
@@ -116,7 +115,6 @@ void InsetBibtex::doDispatch(Cursor & cur, FuncRequest &
cmd)
cur.recordUndo();
setParams(p);
- buffer().invalidateBibfileCache();
buffer().removeBiblioTempFiles();
cur.forceBufferUpdate();
break;
@@ -166,7 +164,7 @@ void InsetBibtex::editDatabases() const
vector<docstring>::const_iterator it = bibfilelist.begin();
vector<docstring>::const_iterator en = bibfilelist.end();
for (; it != en; ++it) {
- FileName const bibfile = getBibTeXPath(*it, buffer());
+ FileName const bibfile = buffer().getBibfilePath(*it);
theFormats().edit(buffer(), bibfile,
theFormats().getFormatFromFile(bibfile));
}
@@ -377,30 +375,9 @@ void InsetBibtex::latex(otexstream & os, OutputParams
const & runparams) const
}
-support::FileNamePairList InsetBibtex::getBibFiles() const
+docstring_list InsetBibtex::getBibFiles() const
{
- FileName path(buffer().filePath());
- support::PathChanger p(path);
-
- // We need to store both the real FileName and the way it is entered
- // (with full path, rel path or as a single file name).
- // The latter is needed for biblatex's central bibfile macro.
- support::FileNamePairList vec;
-
- vector<docstring> bibfilelist =
getVectorFromString(getParam("bibfiles"));
- vector<docstring>::const_iterator it = bibfilelist.begin();
- vector<docstring>::const_iterator en = bibfilelist.end();
- for (; it != en; ++it) {
- FileName const file = getBibTeXPath(*it, buffer());
-
- if (!file.empty())
- vec.push_back(make_pair(*it, file));
- else
- LYXERR0("Couldn't find " + to_utf8(*it) + " in
InsetBibtex::getBibFiles()!");
- }
-
- return vec;
-
+ return getVectorFromString(getParam("bibfiles"));
}
namespace {
@@ -673,11 +650,13 @@ void InsetBibtex::parseBibTeXFiles(FileNameList &
checkedFiles) const
BiblioInfo keylist;
- support::FileNamePairList const files = getBibFiles();
- support::FileNamePairList::const_iterator it = files.begin();
- support::FileNamePairList::const_iterator en = files.end();
- for (; it != en; ++ it) {
- FileName const bibfile = it->second;
+ docstring_list const files = getBibFiles();
+ for (auto const & bf : files) {
+ FileName const bibfile = buffer().getBibfilePath(bf);
+ if (bibfile.empty()) {
+ LYXERR0("Unable to find path for " << bf << "!");
+ continue;
+ }
if (find(checkedFiles.begin(), checkedFiles.end(), bibfile) !=
checkedFiles.end())
// already checked this one. Skip.
continue;
@@ -691,7 +670,6 @@ void InsetBibtex::parseBibTeXFiles(FileNameList &
checkedFiles) const
VarMap strings;
while (ifs) {
-
ifs.get(ch);
if (!ifs)
break;
@@ -850,18 +828,6 @@ void InsetBibtex::parseBibTeXFiles(FileNameList &
checkedFiles) const
}
-FileName InsetBibtex::getBibTeXPath(docstring const & filename, Buffer const &
buf)
-{
- string texfile = changeExtension(to_utf8(filename), "bib");
- // note that, if the filename can be found directly from the path,
- // findtexfile will just return a FileName object for that path.
- FileName file(findtexfile(texfile, "bib"));
- if (file.empty())
- file = FileName(makeAbsPath(texfile, buf.filePath()));
- return file;
-}
-
-
bool InsetBibtex::addDatabase(docstring const & db)
{
docstring bibfiles = getParam("bibfiles");
diff --git a/src/insets/InsetBibtex.h b/src/insets/InsetBibtex.h
index 2d03346..ec783d8 100644
--- a/src/insets/InsetBibtex.h
+++ b/src/insets/InsetBibtex.h
@@ -17,10 +17,10 @@
namespace lyx {
class BiblioInfo;
+class docstring_list;
namespace support {
class FileName;
- class FileNamePairList;
} // namespace support
/** Used to insert BibTeX's information
@@ -33,7 +33,7 @@ public:
~InsetBibtex();
///
- support::FileNamePairList getBibFiles() const;
+ docstring_list getBibFiles() const;
///
bool addDatabase(docstring const &);
///
@@ -78,9 +78,6 @@ public:
//@}
private:
- /// look up the path to the file using TeX
- static support::FileName
- getBibTeXPath(docstring const & filename, Buffer const & buf);
///
void editDatabases() const;
///
diff --git a/src/insets/InsetInclude.cpp b/src/insets/InsetInclude.cpp
index 7b6f389..89f2533 100644
--- a/src/insets/InsetInclude.cpp
+++ b/src/insets/InsetInclude.cpp
@@ -197,11 +197,6 @@ InsetInclude::InsetInclude(InsetInclude const & other)
InsetInclude::~InsetInclude()
{
- if (isBufferLoaded())
- /* We do not use buffer() because Coverity believes that this
- * may throw an exception. Actually this code path is not
- * taken when buffer_ == 0 */
- buffer_->invalidateBibfileCache();
delete label_;
}
@@ -354,8 +349,6 @@ void InsetInclude::setParams(InsetCommandParams const & p)
if (type(params()) == INPUT)
add_preview(*preview_, *this, buffer());
-
- buffer().invalidateBibfileCache();
}
diff --git a/src/support/FileNameList.h b/src/support/FileNameList.h
index 894087d..20b6a85 100644
--- a/src/support/FileNameList.h
+++ b/src/support/FileNameList.h
@@ -28,16 +28,6 @@ class FileNameList: public std::vector<FileName>
{
};
-
-/**
- * List of filename with additional information. Used by the Bibfiles cache,
- * which needs to store, next to the real filename, the way it was entered
- * in the BibTeX inset (as abspath, relpath or texmf file)
- */
-class FileNamePairList: public std::vector<std::pair<docstring, FileName>>
-{
-};
-
} // namespace support
} // namespace lyx
