Le 14/10/2015 21:35, Georg Baum a écrit :
*#2: Add a unique id to math insets. I also print this information in
the DEVEL_VERSION in the status bar similarly to what is done currently
for paragraphs. It is interesting that while the paragraph id is usually
below 10000, this new math inset id immediately goes into the millions
on opening an actual LyX document and it increases very quickly (which I
suspect is due to updateMacros()).
Please measure memory consumption. I am a bit afraid that this increases
memory usage a lot, since math insets contain very tiny bits of information,
and you may need _lots_ of them even for simple formula.
I did not realise that there were supposed to be so many InsetMaths live
at the same time. Although the increased memory use is quite small for
modern standard, I agree that it is a bad idea to increase it needlessly
both in principle and in the long run.
In fact I only need uids on InsetMathNest, which should represent a much
smaller population.
Why do you need a new member variable at all? Why can't you just use the
memory address? This would not consume memory and still be a unique id.
Yes, it would make sense to use pointers IMO. There was two reasons for
the UniqueId class: I needed to experiment with different copy semantics
before I finally decided for the current behaviour; and also I find that
using the address looked kludgy compared to the current code (e.g. the
paragraph id). Another aspect is that during debugging, having an id
that starts from 0 is easier to read and keep track of. I suggest seeing
how it goes with having ids only for InsetMathNest. Does that make sense?
*#6: Remove a deep copy of MathData in lyx::write that interfered with
TexRow. If anybody can explain to me the purpose of this antediluvian
call to extractStrings, I am all ears.
Fortunately I can answer this question: extractStrings() exists to make the
formulas better suitable for feeding them to external programs like computer
algebra systems. It is also used for debug output (dump()) to make it more
readable for humans.
Thank you for the explanation.
In the meanwhile, I wrote
equivalent code in case it was actually useful.
The new code looks rather complicated to me. I think the old one should be
kept.
ExtractStrings forces write to repeat a deep copy on a recursive call. I
would advise against keeping write as it is. There already was an
unexplained time explosion when generating previews in a document with a
lot of math macros, before Enrico rationalised the macro definition
output during preview. What was not explained is that it took more than
a time linear in the size of the generated preview tex file, in the
tests that I did at the time. Not to say that it's necessarily related
(though it may be), but this can be a concrete issue.
I am skeptical about "complicated" given that it is the same as before,
except that it was doing a deep copy before (which I call "complicated"
in this context). As for readability, see the attached diff where it is
rephrased using a separate function.
What I do not understand is why this code is executed at all for LaTeX
output. I believe that it can even produce wrong PDF contents.
Do you have an example of how it might invalidate the latex output?
Therefore, if
LaTeX output is fixed to not call the free function write() at all, then
extractStrings() can be kept and will not interfere.
Sorry, that's beyond my current knowledge of LyX internals. I am
confident in my patches because I make small local changes. I suggest
keeping my fix for now, made more readable as attached, unless somebody
is able to provide a patch along your lines in a timely manner.
I noticed a major
decrease of the math inset ids after this patch; hopefully this id is a
new metrics that will allow us to observe further improvements to the
implementation of maths.
I am not sure whether this is a good measure for mathed code quality. It is
a central idea of mathed that a math inset is a self contained data class.
If you assume that then it is not necessarily a problem if many copies are
made.
You might be right; I found interesting though to observe such a
decrease in the uid counter with the patch discussed above.
Guillaume
diff --git a/src/mathed/MathExtern.cpp b/src/mathed/MathExtern.cpp
index 1904843..afd224a 100644
--- a/src/mathed/MathExtern.cpp
+++ b/src/mathed/MathExtern.cpp
@@ -193,6 +193,23 @@ void extractStrings(MathData & ar)
}
+//In-place version of extractStrings. Returns false if there is no string to
+//extract. Otherwise, the extracted string is appended to s and the iterator
+//"from" is incremented appropriately.
+bool extractString(MathData::const_iterator & from,
+ MathData::const_iterator const & to,
+ docstring& s)
+{
+ bool ret = false;
+ for (; from != to && (*from)->asCharInset(); ++from) {
+ ret = true;
+ s += (*from)->getChar();
+ }
+ return ret;
+}
+
+
+
void extractMatrices(MathData & ar)
{
//lyxerr << "\nMatrices from: " << ar << endl;
@@ -1382,11 +1399,17 @@ namespace {
void write(MathData const & dat, WriteStream & wi)
{
- MathData ar = dat;
- extractStrings(ar);
wi.firstitem() = true;
- for (MathData::const_iterator it = ar.begin(); it != ar.end(); ++it) {
- (*it)->write(wi);
+ MathData::const_iterator const end = dat.end();
+ for (MathData::const_iterator it = dat.begin(); it != end; ++it) {
+ // extracting strings when possible
+ docstring s;
+ if (extractString(it, end, s)) {
+ InsetMathString ms(s);
+ ms.write(wi);
+ } else {
+ (*it)->write(wi);
+ }
wi.firstitem() = false;
}
}
