Now, in addRules we call addRule, which emit rulesChanged, that leads to rehighlight all worksheet. This is useless work, because for each rule in rules container we reprocessing all worksheet. (instead better option to emit rulesChanged only one time after container processing). So, when container is big, performance falls quite strongly, for example highlighter of octave backend spends 2-4 seconds for adding new function. So this changes improve emit logic, calling only one event for rules container.
Details
Details
- Reviewers
asemke - Group Reviewers
Cantor - Commits
- R55:16819f289de9: Fix bad performance for `addRules` in `DefaultHighlighter` for big containers
- Start octave backend, write octave function, check, that cantor is freezed for few seconds
- Apply patch
- Check, that octave backend don't freeze after entry with user function.
Diff Detail
Diff Detail
- Repository
- R55 Cantor
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Comment Actions
Can we try to implement this with a boolean paramer m_suppressRuleChangedSignal instead of adding new functions? In addRules() we set m_suppressRuleChangedSignal = true and use in addRule()
if (!m_supressRuleChangedSignal)
emit rulesChanged();The code will be more compact and clean with this. Also, this would be similar to the convention we use in LabPlot.
Can you please provide your octave functions which you used to test with?
P.S. : the template for DefaultHighlighter::addRules() in defaulthighlighter.h looks like an overkill - this function is called only twice in the code and the container is always QStringList. The same is true most probably for other templated add*() funcitons in defaulthighlighter.h. We should simplify this maybe.
Comment Actions
Without this patch, it takes 4-5 seconds to run this 5 entries.
#Entry 1
function nothing(n)
printf("do nothing");
endfunction;
#Entry 2
function nothing1(n)
printf("do nothing");
endfunction;
#Entry 3
function nothing2(n)
printf("do nothing");
endfunction;
#Entry 4
function nothing3(n)
printf("do nothing");
endfunction;
#Entry 5
function nothing4(n)
printf("do nothing");
endfunction;Without this patch, it takes 4-5 seconds to run this 5 entries.
P.S. : the template for DefaultHighlighter::addRules() in defaulthighlighter.h looks like an overkill - this function is called only twice in the code and the container is always QStringList. The same is true most probably for other templated add*() funcitons in defaulthighlighter.h. We should simplify this maybe.
I agree, that we should simplify this templates methods to not template functions, which acepting QStringList. Should I add it to this diff?
Comment Actions
This is not exactly what I meant. bool m_suppressRuleChangeSignal is a private class member (that's why m_ in the name) which is initialized in the constructor to false. We don't need to change the function signatures. We only add
m_suppressRuleChangedSignal = true;
for (;i != end; ++i)
addRule(*i, format);
m_suppressRuleChangedSignal = false;in DefaultHighlither::addRules() and that
if (!m_suppressRuleChangedSignal)
emit ruleChanged();that you already have now in DefaultHighlighter::addRule(). And similar for removeRule-stuff. No need to change the signatures of the already available functions.
Since we don't have any private members in DefaultHighlighter except of the add pointers, simply add bool suppressRuleChangedSignal to DefaultHighlighterPrivate instead of adding m_suppressRuleChangedSignal to DefaultHighlighter and use it via d->suppressRuleChangedSignal.
Comment Actions
I agree, that we should simplify this templates methods to not template functions, which acepting QStringList. Should I add it to this diff?
Yes, please. Let's do the clean-up here in the same diff.
Comment Actions
Your diff seems to be wrong. Looks like you calculated the diff between the current version and the temporary version you had before with m_supressRuleChangedSignal. Can you please upload the corrected diff?