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 6014492699)
This commit is contained in:
Richard Kimberly Heck 2018-09-02 00:10:01 -04:00
parent 8c32a1a7b9
commit 7c614cc6b7
7 changed files with 119 additions and 109 deletions

View File

@ -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);

View File

@ -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;

View File

@ -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;
}

View File

@ -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");

View File

@ -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;
///

View File

@ -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();
}

View File

@ -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