Patch comments.
> From 7fd53bc44bd278fd1f30400c030e350f2c0a6aa3 Mon Sep 17 00:00:00 2001
> From: Guillaume Munch <g...@lyx.org>
> Date: Sun, 31 Jul 2016 00:42:51 +0100
> Subject: [PATCH] Replace static with thread_local when used for caching
>
> thread_local is a per-thread static variable, so it is thread-safe and can be
> used for caching purpose.
>
> (requires gcc >= 4.8)
> ---
> src/frontends/qt4/GuiCitation.cpp | 11 ++++++++---
> src/frontends/qt4/GuiDocument.cpp | 7 ++++++-
> src/frontends/qt4/GuiFontLoader.cpp | 6 +++++-
> src/frontends/qt4/GuiInclude.cpp | 12 ++++++++----
> src/frontends/qt4/GuiListings.cpp | 9 +++++++--
> src/frontends/qt4/GuiPainter.cpp | 7 ++++++-
> src/frontends/qt4/GuiSymbols.cpp | 5 ++++-
> 7 files changed, 44 insertions(+), 13 deletions(-)
>
> diff --git a/src/frontends/qt4/GuiCitation.cpp
> b/src/frontends/qt4/GuiCitation.cpp
> index 028e874..f209a24 100644
> --- a/src/frontends/qt4/GuiCitation.cpp
> +++ b/src/frontends/qt4/GuiCitation.cpp
> @@ -570,12 +570,17 @@ void GuiCitation::findKey(BiblioInfo const & bi,
> docstring field, docstring entry_type,
> bool case_sensitive, bool reg_exp, bool reset)
> {
> - // FIXME THREAD
> - // Used for optimisation: store last searched string.
> +#if defined(__GNUC__) && (__GNUC__ == 4) && (__GNUC_MINOR__ == 6)
> static QString last_searched_string;
> - // Used to disable the above optimisation.
> static bool last_case_sensitive;
> static bool last_reg_exp;
> +#else
> + // Used for optimisation: store last searched string.
> + thread_local QString last_searched_string;
> + // Used to disable the above optimisation.
> + thread_local bool last_case_sensitive;
> + thread_local bool last_reg_exp;
> +#endif
Is there some way we could store these per BufferView or something? I'd
kind of
like to save the last searched string for each document, rather than
globally.
> diff --git a/src/frontends/qt4/GuiDocument.cpp
> b/src/frontends/qt4/GuiDocument.cpp
> index e116001..c98332e 100644
> --- a/src/frontends/qt4/GuiDocument.cpp
> +++ b/src/frontends/qt4/GuiDocument.cpp
> @@ -1498,9 +1498,14 @@ QString GuiDocument::validateListingsParameters()
> {
> // use a cache here to avoid repeated validation
> // of the same parameters
> - // FIXME THREAD
> + // FIXME code duplication with GuiInclude and GuiListings
> +#if defined(__GNUC__) && (__GNUC__ == 4) && (__GNUC_MINOR__ == 6)
> static string param_cache;
> static QString msg_cache;
> +#else
> + thread_local string param_cache;
> + thread_local QString msg_cache;
> +#endif
I think this cache could just be removed. It's used when we validate
listings parameters,
which is (a) very rarely and (b) a GUI-related operation, which means
that the very minor
speed optimization involved will not be noticed.
> return QString();
> diff --git a/src/frontends/qt4/GuiFontLoader.cpp
> b/src/frontends/qt4/GuiFontLoader.cpp
> index cc092f5..45392d8 100644
> --- a/src/frontends/qt4/GuiFontLoader.cpp
> +++ b/src/frontends/qt4/GuiFontLoader.cpp
> @@ -373,9 +373,13 @@ GuiFontInfo::GuiFontInfo(FontInfo const & f)
>
> bool FontLoader::available(FontInfo const & f)
> {
> - // FIXME THREAD
> +#if defined(__GNUC__) && (__GNUC__ == 4) && (__GNUC_MINOR__ == 6)
> static vector<int> cache_set(NUM_FAMILIES, false);
> static vector<int> cache(NUM_FAMILIES, false);
> +#else
> + thread_local vector<int> cache_set(NUM_FAMILIES, false);
> + thread_local vector<int> cache(NUM_FAMILIES, false);
> +#endif
Here, I think we'd prefer to have something actually global (although,
as I said, this
change will not now have any practical effect). Can the "atomic" stuff
be used to allow
us to do that?
PS We have:
static vector<int> cache_set(NUM_FAMILIES, false);
cache_set[family] = true;
??
> diff --git a/src/frontends/qt4/GuiInclude.cpp
> b/src/frontends/qt4/GuiInclude.cpp
> index e1fc275..2fe7f8f 100644
> --- a/src/frontends/qt4/GuiInclude.cpp
> +++ b/src/frontends/qt4/GuiInclude.cpp
> @@ -93,10 +93,14 @@ docstring GuiInclude::validate_listings_params()
> {
> // use a cache here to avoid repeated validation
> // of the same parameters
> - // FIXME THREAD
> - static string param_cache = string();
> - static docstring msg_cache = docstring();
> -
> +#if defined(__GNUC__) && (__GNUC__ == 4) && (__GNUC_MINOR__ == 6)
> + static string param_cache;
> + static docstring msg_cache;
> +#else
> + thread_local string param_cache;
> + thread_local docstring msg_cache;
> +#endif
> +
As above, I think this cachee could just be removed.
> diff --git a/src/frontends/qt4/GuiListings.cpp
> b/src/frontends/qt4/GuiListings.cpp
> index da12882..b9edeb3 100644
> --- a/src/frontends/qt4/GuiListings.cpp
> +++ b/src/frontends/qt4/GuiListings.cpp
> @@ -348,10 +348,15 @@ docstring GuiListings::validate_listings_params()
> {
> // use a cache here to avoid repeated validation
> // of the same parameters
> - // FIXME THREAD
> + //
> +#if defined(__GNUC__) && (__GNUC__ == 4) && (__GNUC_MINOR__ == 6)
> static string param_cache;
> static docstring msg_cache;
> -
> +#else
> + thread_local string param_cache;
> + thread_local docstring msg_cache;
> +#endif
> +
And here.
Rest looks good.
Richard
