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.

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 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
@@ -2394,7 +2394,7 @@ void Buffer::invalidateBibinfoCache() const
 }
 
 
-FileNamePairList const & Buffer::getBibfiles(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 instead.
@@ -2420,7 +2420,7 @@ BiblioInfo const & Buffer::bibInfo() const
 }
 
 
-void Buffer::registerBibfiles(FileNamePairList const & bf) 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,
@@ -2430,7 +2430,7 @@ void Buffer::registerBibfiles(FileNamePairList const & bf) const
 		tmp->registerBibfiles(bf);
 
 	for (auto const & p : bf) {
-		FileNamePairList::const_iterator temp =
+		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);
@@ -2438,6 +2438,31 @@ void Buffer::registerBibfiles(FileNamePairList const & bf) const
 }
 
 
+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
@@ -2453,11 +2478,9 @@ void Buffer::checkIfBibInfoCacheIsValid() const
 		return;
 
 	// compare the cached timestamps with the actual ones.
-	FileNamePairList const & bibfiles_cache = getBibfiles();
+	docstring_list const & bibfiles_cache = getBibfiles();
-	FileNamePairList::const_iterator ei = bibfiles_cache.begin();
+	for (auto const & bf : bibfiles_cache) {
-	FileNamePairList::const_iterator en = bibfiles_cache.end();
+		FileName const fn = getBibfilePath(bf);
-	for (; ei != en; ++ ei) {
-		FileName const fn = ei->second;
 		time_t lastw = fn.lastModified();
 		time_t prevw = d->bibfile_status_[fn];
 		if (lastw != prevw) {
@@ -2478,10 +2501,17 @@ void Buffer::reloadBibInfoCache(bool const force) const
 		return;
 	}
 
-	checkIfBibInfoCacheIsValid();
+	if (!force) {
-	if (d->bibinfo_cache_valid_ && !force)
+		checkIfBibInfoCacheIsValid();
-		return;
+		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);
@@ -3197,7 +3227,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
@@ -3219,7 +3249,7 @@ vector<docstring> const Buffer::prepareBibFilePaths(OutputParams const & runpara
 	bool found_space = false;
 
 	for (auto const & bit : bibfilelist) {
-		string utf8input = to_utf8(bit.first);
+		string utf8input = to_utf8(bit);
 		string database =
 			prepareFileNameForLaTeX(utf8input, ".bib", runparams.nice);
 		FileName try_in_file =
@@ -3228,7 +3258,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 = bit.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));
@@ -4770,7 +4800,7 @@ void Buffer::updateBuffer(UpdateScope scope, UpdateType utype) const
 	Buffer const * const master = masterBuffer();
 	DocumentClass const & textclass = master->params().documentClass();
 
-	FileNamePairList old_bibfiles;
+	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) {
@@ -4834,7 +4864,7 @@ void Buffer::updateBuffer(UpdateScope scope, UpdateType utype) const
 		return;
 
 	// if the bibfiles changed, the cache of bibinfo is invalid
-	FileNamePairList new_bibfiles = d->bibfiles_cache_;
+	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());

src/Buffer.h

@@ -70,7 +70,6 @@ class WorkAreaManager;
 namespace support {
 class DocFileName;
 class FileName;
-class FileNamePairList;
 } // namespace support
 
 namespace graphics {
@@ -417,7 +416,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.
@@ -765,7 +764,9 @@ public:
 	bool areChangesPresent() const;
 	void updateChangesPresent() const;
 	///
-	void registerBibfiles(support::FileNamePairList const & bf) const;
+	void registerBibfiles(docstring_list const & bf) const;
+	///
+	support::FileName getBibfilePath(docstring const & bibid) const;
 
 private:
 	friend class MarkAsExporting;
@@ -784,7 +785,7 @@ private:
 	void checkIfBibInfoCacheIsValid() const;
 	/// Return the list with all bibfiles in use (including bibfiles
 	/// of loaded child documents).
-	support::FileNamePairList const &
+	docstring_list const &
 		getBibfiles(UpdateScope scope = UpdateMaster) const;
 	///
 	void collectChildren(ListOfBuffers & children, bool grand_children) const;

src/insets/InsetBibtex.cpp

@@ -38,6 +38,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"
@@ -151,7 +152,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));
 	}
@@ -382,29 +383,9 @@ void InsetBibtex::latex(otexstream & os, OutputParams const & runparams) const
 }
 
 
-FileNamePairList InsetBibtex::getBibFiles() const
+docstring_list InsetBibtex::getBibFiles() const
 {
-	FileName path(buffer().filePath());
+	return getVectorFromString(getParam("bibfiles"));
-	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.
-	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;
 }
 
 namespace {
@@ -675,11 +656,13 @@ void InsetBibtex::parseBibTeXFiles(FileNameList & checkedFiles) const
 
 	BiblioInfo keylist;
 
-	FileNamePairList const files = getBibFiles();
+	docstring_list const files = getBibFiles();
-	FileNamePairList::const_iterator it = files.begin();
+	for (auto const & bf : files) {
-	FileNamePairList::const_iterator en = files.end();
+		FileName const bibfile = buffer().getBibfilePath(bf);
-	for (; it != en; ++ it) {
+		if (bibfile.empty()) {
-		FileName const bibfile = it->second;
+			LYXERR0("Unable to find path for " << bf << "!");
+			continue;
+		}
 		if (find(checkedFiles.begin(), checkedFiles.end(), bibfile) != checkedFiles.end())
 			// already checked this one. Skip.
 			continue;
@@ -697,7 +680,6 @@ void InsetBibtex::parseBibTeXFiles(FileNameList & checkedFiles) const
 		VarMap strings;
 
 		while (ifs) {
-
 			ifs.get(ch);
 			if (!ifs)
 				break;
@@ -856,18 +838,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");

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
@@ -31,7 +31,7 @@ public:
 	InsetBibtex(Buffer *, InsetCommandParams const &);
 
 	///
-	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;
 	///

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