Re: [LyX/2.2.x] Start preparations for 2.2.1.

[Guenter Milde]

On 2016-07-30, Guillaume Munch wrote:
> Le 26/07/2016 à 09:23, Guenter Milde a écrit :
> (I have a similar issue with the "source panel": it should really be
> called "output panel".

>> I cannot follow this argument: the "source panel" shows various output
>> formats (TeX/HTML/LyX) in source form - (as opposed to a user agent
>> (browser) showing HTML in a presentational form or the LyX GUI window
>> showing the source in presentational/interactive form.

> "Source" depends on the context, and in LyX one would expect that it 
> refers to some source for LyX.

As the LyX-source format is one of the several source formats that can be
shown in the source panel, this is no problem.

In the context of editing a document that can be exported to several other
source formats, showing these other sources is not far off either...


> What about "Code preview" ? inspired by the current French translation.

"Code" or "source code" is usually the source for a program. LyX is a
document processor where the source is text in a markup language.

The source panel shows documents in several markup languages (HTML, LaTeX,
LyX), not Code.

Günter

Re: Pandora's box

[Richard Heck]

Patch comments.

> From 7fd53bc44bd278fd1f30400c030e350f2c0a6aa3 Mon Sep 17 00:00:00 2001
> From: Guillaume Munch 
> 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 cache_set(NUM_FAMILIES, false);
>   static vector cache(NUM_FAMILIES, false);
> +#else
> + thread_local vector cache_set(NUM_FAMILIES, false);
> + thread_local vector 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 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 

Re: Pandora's box

[Richard Heck]

On 07/31/2016 01:38 PM, Guillaume Munch wrote:
> Hi list,
>
> By touching upon the problem of static variables and threading in LyX
> (see my patch at ), I
> have the impression that I opened Pandora's box.

I think the URL there is broken.

> I just committed a series of patches that fixes "FIXME thread" in cases
> where c++11 gives an easy solution for common problems, with some
> explanations which you may enjoy.

Thanks. There are surely a lot of hidden problems here. The LyX code was
not originally written with thread-safety in mind. It was only when
background
export was added that these issues became relevant.

> For other "FIXME thread" issues, either the solution is more complex, or
> using statics is inappropriate altogether (I think). Is there a plan to
> get rid of these?

It looks to me as if most of the ones that are left are in
src/frontends/. It would
surely be worth fixing these, but I doubt they actually cause problems,
at least as
the code is now. We only ever have one GUI thread. For thar reason, the
switch
from static to thread_local in GuiDocument, say, will have no practical
effect at
the moment.

> Also, when reviewing the use of non-const static variables, I found many
> cases where thread-safety was not obvious to me but no "FIXME thread"
> comment was present. Is it known whether the remaining static variables
> without "FIXME thread" are thread-safe?

I went through and marked these some time ago. I believe I checked all
static
variables, and the ones I did not mark I guess I thought were safe. I
ought to
have added a comment in such cases.

> Ideally, I think it would be nice to only use statics when necessary,
> and use the new tools provided by C++11 (see my recent commits and the
> attached patch).

Also probably worth doing.

Richard

Re: [LyX/master] Perf comments: getFormatFromFile slows LyX startup considerably

[Richard Heck]

On 07/31/2016 01:36 PM, Guillaume Munch wrote:
> commit 0a8b7f6a5740f4b4f976eb308841ad3b98166015
> Author: Guillaume Munch 
> Date:   Thu Jul 28 16:52:14 2016 +0100
>
> Perf comments: getFormatFromFile slows LyX startup considerably
> 
> It is currently called on hundreds of files: settings, layouts, icons, 
> cached
> graphics files (incl. graphics from files that are not opened on startup).

Regarding zipped files, perhaps we could check the extension and assume
it is zipped or not, on that basis. Then if something goes wrong, we
could try getFormatFromFile?

Richard

Pandora's box

[Guillaume Munch]

Hi list,


By touching upon the problem of static variables and threading in LyX
(see my patch at ), I
have the impression that I opened Pandora's box.

I just committed a series of patches that fixes "FIXME thread" in cases
where c++11 gives an easy solution for common problems, with some
explanations which you may enjoy.

Here's an additional patch that uses thread_local instead of static to
fix, I think, threading issues for some of the cases (e.g. when caching
some value for optimisation purpose). But I'd like to hear some comments
before committing.

For other "FIXME thread" issues, either the solution is more complex, or
using statics is inappropriate altogether (I think). Is there a plan to
get rid of these?

Also, when reviewing the use of non-const static variables, I found many
cases where thread-safety was not obvious to me but no "FIXME thread"
comment was present. Is it known whether the remaining static variables
without "FIXME thread" are thread-safe?

Ideally, I think it would be nice to only use statics when necessary,
and use the new tools provided by C++11 (see my recent commits and the
attached patch).


Guillaume
>From 7fd53bc44bd278fd1f30400c030e350f2c0a6aa3 Mon Sep 17 00:00:00 2001
From: Guillaume Munch 
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
 	// Reset last_searched_string in case of changed option.
 	if (last_case_sensitive != case_sensitive
 		|| last_reg_exp != reg_exp) {
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
 
 	if (listingsModule->bypassCB->isChecked())
 		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 cache_set(NUM_FAMILIES, false);
 	static vector cache(NUM_FAMILIES, false);
+#else
+	thread_local vector cache_set(NUM_FAMILIES, false);
+	thread_local vector cache(NUM_FAMILIES, false);
+#endif
 
 	FontFamily family = f.family();
 #ifdef Q_OS_MAC
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 

Re: Unknown tag in

[Richard Heck]

On 07/31/2016 06:00 AM, Kornel Benko wrote:
> I am getting now following warning starting lyx:
> LyX: Unknown tag `'' [around line 409 of file ~/.lyx2/lyxrc.defaults current 
> token: ''' context: '']
>
> The line reads:
>   \converter diaeps"dia -e $$o -t eps $$i"""'
> (including the \' at end of line)
>
> Is this a typo?

Yes, fixed. Thanks for noticing.

Richard

Unknown tag in

[Kornel Benko]

I am getting now following warning starting lyx:
LyX: Unknown tag `'' [around line 409 of file ~/.lyx2/lyxrc.defaults current 
token: ''' context: '']

The line reads:
\converter diaeps"dia -e $$o -t eps $$i"""'
(including the \' at end of line)

Is this a typo?
lib/configure.py:980
if dia == 'dia':
addToRC(r'''\converter diapng"dia -e $$o -t png $$i"
""
\converter diaeps"dia -e $$o -t eps $$i"""'
\converter diasvg"dia -e $$o -t svg $$i"""''')

Kornel



signature.asc
Description: This is a digitally signed message part.

Remaining failures on xhtml export (tested with xmllint)

[Kornel Benko]

After this many fixes from Richard I felt the desire to test if
some (or all) of the xhtml export tests are now successful.
There are some remaining cases (23 out of 357 tested).

Here the list as regular expressions to make the list shorter:
..
export/doc/attic/eu_UserGuide_xhtml
export/doc/de/Additional_xhtml
export/doc/(de|es|fr|ja)/UserGuide_xhtml
export/examples/beamer(|-article|lyxexample1|poster)_xhtml
export/examples/powerdot-example_xhtml
export/examples/script_form_xhtml
export/examples/spreadsheet_xhtml
export/examples/(de|fr)/beamer-article_xhtml
export/examples/(de|fr|ja)/beamer_xhtml
export/templates/(|de_|es_|fr_|ja|)beamer-conference-ornate-20min_xhtml

I weakly remember that there was previously over 40 tests failing, so this is a 
great improvement.

Kornel

signature.asc
Description: This is a digitally signed message part.

Re: External Material Problems

[Richard Heck]

On 07/30/2016 11:37 PM, Richard Heck wrote:
> There seem to be some general problems concerning external material and
> the temporary directory.
>
> First, preview of external material does not seem to work. Constructs like:
>
> Format DocBook
> Product " fileref=\"$$AbsOrRelPathMaster$$Basename.eps\">"
> UpdateFormat eps
> UpdateResult "$$AbsPath$$Basename.eps"
> ReferencedFile docbook "$$AbsPath$$Basename.eps"
> ReferencedFile docbook-xml "$$AbsPath$$Basename.eps"
> FormatEnd
>
> fail on preview, because it is looking for the exported filename, not
> the mangled one in the temporary directory. Similarly, constructs like
>
> Format Ascii
> Product "$$Contents(\"$$AbsPath$$Basename.asc\")"
> UpdateFormat asciichess
> UpdateResult "$$AbsPath$$Basename.asc"
> FormatEnd
>
> fail, for the same reason: They try to include a file in the document
> directory.

I fixed this.

rh