D21788: Make Plasma::Svg::elementRect a bit leaner

This revision was automatically updated to reflect the committed changes. Closed by commit R242:097f0e83e7bf: Make Plasma::Svg::elementRect a bit leaner (authored by apol). REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21788?vs=59837=60234

D21788: Make Plasma::Svg::elementRect a bit leaner

davidedmundson accepted this revision. This revision is now accepted and ready to land. REPOSITORY R242 Plasma Framework (Library) BRANCH master REVISION DETAIL https://phabricator.kde.org/D21788 To: apol, #plasma, #frameworks, davidedmundson Cc: bruns, kde-frameworks-devel, LeGast00n,

D21788: Make Plasma::Svg::elementRect a bit leaner

apol added a comment. ping? REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D21788 To: apol, #plasma, #frameworks Cc: bruns, kde-frameworks-devel, LeGast00n, michaelh, ngraham

D21788: Make Plasma::Svg::elementRect a bit leaner

apol marked an inline comment as done. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D21788 To: apol, #plasma, #frameworks Cc: bruns, kde-frameworks-devel, LeGast00n, michaelh, ngraham

D21788: Make Plasma::Svg::elementRect a bit leaner

apol updated this revision to Diff 59837. apol added a comment. static const to be sure that QRegularExpression is only initialised once REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21788?vs=59772=59837 BRANCH master REVISION

D21788: Make Plasma::Svg::elementRect a bit leaner

bruns added inline comments. INLINE COMMENTS > apol wrote in svg.cpp:123 > pcre2 does it, AFAIK. Benchmark tells `const static` is faster: F6890682: qre_bench.cpp REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL

D21788: Make Plasma::Svg::elementRect a bit leaner

apol added inline comments. INLINE COMMENTS > bruns wrote in svg.cpp:123 > Are you sure about this? As far as I can see: > > `QRegularExpression::QRegularExpression(QString pattern)` default constructs > a `QRegularExpressionPrivate`, which sets `dirty(true)`. > > Calling `QRE::globalMatch()`

D21788: Make Plasma::Svg::elementRect a bit leaner

bruns added inline comments. INLINE COMMENTS > apol wrote in svg.cpp:123 > static isn't necessary, QRegularExpression already has internal regex > compilation optimisations we can leverage. > Does the assert bother you? Are you sure about this? As far as I can see:

D21788: Make Plasma::Svg::elementRect a bit leaner

apol marked an inline comment as done. apol added inline comments. INLINE COMMENTS > bruns wrote in svg.cpp:123 > Are you sure you uploaded the right diff? No const static, Q_ASSERT ... static isn't necessary, QRegularExpression already has internal regex compilation optimisations we can

D21788: Make Plasma::Svg::elementRect a bit leaner

bruns added inline comments. INLINE COMMENTS > svg.cpp:123 > +QRegularExpression > idExpr(QLatin1String("id\\s*?=\\s*?(['\"])(\\d+?-\\d+?-.*?)\\1")); > +Q_ASSERT(idExpr.isValid()); > Are you sure you uploaded the right diff? No const static, Q_ASSERT ... REPOSITORY R242 Plasma

D21788: Make Plasma::Svg::elementRect a bit leaner

apol updated this revision to Diff 59772. apol added a comment. Prefer a QRegularExpression to QRegExp REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21788?vs=59746=59772 BRANCH master REVISION DETAIL

D21788: Make Plasma::Svg::elementRect a bit leaner

bruns added a comment. Please put the dead code removal in a separate review. INLINE COMMENTS > svg.cpp:111 > const QString contentsAsString(QString::fromLatin1(contents)); > -QRegExp idExpr(QLatin1String("id\\s*=\\s*(['\"])(\\d+-\\d+-.*)\\1")); > +static QRegExp >

D21788: Make Plasma::Svg::elementRect a bit leaner

apol created this revision. apol added reviewers: Plasma, Frameworks. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. apol requested review of this revision. REVISION SUMMARY Don't double-access maps Don't construct ids more times than necessary. Make