From 60506b878e151aeb7e20de3d33e5611ea40c5ae6 Mon Sep 17 00:00:00 2001 From: Benoit Pierre Date: Tue, 24 Oct 2023 19:12:37 +0200 Subject: [PATCH] avoid unnecessary string modifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/koreader/crengine/pull/543: - lvstring: drop `& operator []` accessors, as they end up always being used, even for read-only lookup, triggering unnecessary calls to `modify()` strings. - avoid unnecessary use of `lStringXX:at(…)` (which could trigger a call to `modify()`). - lvstring: fix `lastChar/firstChar` implementations: don't use `at()`, which could trigger a call to `modify()`. - lvstring: drop `at()` methods: not used anymore. --- crengine/include/lvstring.h | 16 ++++------------ crengine/src/lvrend.cpp | 8 ++++---- crengine/src/lvtinydom.cpp | 2 +- crengine/src/mathml.cpp | 30 ++++++++++++++++-------------- 4 files changed, 25 insertions(+), 31 deletions(-) diff --git a/crengine/include/lvstring.h b/crengine/include/lvstring.h index 29c96360a..941f49474 100644 --- a/crengine/include/lvstring.h +++ b/crengine/include/lvstring.h @@ -553,19 +553,15 @@ class lString8 bool startsWith ( const char * substring ) const; /// returns last character - value_type lastChar() { return empty() ? 0 : at(length()-1); } + value_type lastChar() const { return empty() ? 0 : pchunk->buf8[length()-1]; } /// returns first character - value_type firstChar() { return empty() ? 0 : at(0); } + value_type firstChar() const { return empty() ? 0 : pchunk->buf8[0]; } /// calculate hash lUInt32 getHash() const; - /// get character at specified position with range check - value_type & at( size_type pos ) { if (pos > (size_type)pchunk->len) crFatalError(); return modify()[pos]; } /// get character at specified position without range check value_type operator [] ( size_type pos ) const { return pchunk->buf8[pos]; } - /// get reference to character at specified position - value_type & operator [] ( size_type pos ) { return modify()[pos]; } /// ensures that reference count is 1 void lock( size_type newsize ); @@ -817,18 +813,14 @@ class lString32 bool startsWithNoCase (const lString32 & substring) const; /// returns last character - value_type lastChar() { return empty() ? 0 : at(length()-1); } + value_type lastChar() const { return empty() ? 0 : pchunk->buf32[length()-1]; } /// returns first character - value_type firstChar() { return empty() ? 0 : at(0); } + value_type firstChar() const { return empty() ? 0 : pchunk->buf32[0]; } /// calculates hash for string lUInt32 getHash() const; - /// returns character at specified position, with index bounds checking, fatal error if fails - value_type & at( size_type pos ) { if ((unsigned)pos > (unsigned)pchunk->len) crFatalError(); return modify()[pos]; } /// returns character at specified position, without index bounds checking value_type operator [] ( size_type pos ) const { return pchunk->buf32[pos]; } - /// returns reference to specified character position (lvalue) - value_type & operator [] ( size_type pos ) { return modify()[pos]; } /// resizes string, copies if several references exist void lock( size_type newsize ); /// returns writable pointer to string buffer diff --git a/crengine/src/lvrend.cpp b/crengine/src/lvrend.cpp index cb4a87b3f..4d4b198db 100644 --- a/crengine/src/lvrend.cpp +++ b/crengine/src/lvrend.cpp @@ -1755,7 +1755,7 @@ class CCRTable { parent = parent->getParentNode(); if ( parent && parent->hasAttribute(LXML_NS_ANY, attr_href ) ) { lString32 href = parent->getAttributeValue(LXML_NS_ANY, attr_href ); - if ( href.length()>0 && href.at(0)=='#' ) { + if ( href.firstChar()=='#' ) { href.erase(0,1); if ( is_single_column ) row->single_col_context->addLink( href, link_insert_pos ); @@ -5281,7 +5281,7 @@ int renderBlockElementLegacy( LVRendPageContext & context, ldomNode * enode, int // specifying a -cr-hint: to the footnote target, with no need // to set one to the link itself lString32 href = parent->getAttributeValue(LXML_NS_ANY, attr_href ); - if ( href.length()>0 && href.at(0)=='#' ) { + if ( href.firstChar()=='#' ) { href.erase(0,1); context.addLink( href, link_insert_pos ); } @@ -5316,7 +5316,7 @@ int renderBlockElementLegacy( LVRendPageContext & context, ldomNode * enode, int parent = parent->getParentNode(); if ( parent && parent->hasAttribute(LXML_NS_ANY, attr_href ) ) { lString32 href = parent->getAttributeValue(LXML_NS_ANY, attr_href ); - if ( href.length()>0 && href.at(0)=='#' ) { + if ( href.firstChar()=='#' ) { href.erase(0,1); context.addLink( href, link_insert_pos ); } @@ -8722,7 +8722,7 @@ void renderBlockElementEnhanced( FlowState * flow, ldomNode * enode, int x, int // specifying a -cr-hint: to the footnote target, with no need // to set one to the link itself lString32 href = parent->getAttributeValue(LXML_NS_ANY, attr_href ); - if ( href.length()>0 && href.at(0)=='#' ) { + if ( href.firstChar()=='#' ) { href.erase(0,1); flow->getPageContext()->addLink( href, link_insert_pos ); } diff --git a/crengine/src/lvtinydom.cpp b/crengine/src/lvtinydom.cpp index d95f3882d..fd76bf47e 100644 --- a/crengine/src/lvtinydom.cpp +++ b/crengine/src/lvtinydom.cpp @@ -14790,7 +14790,7 @@ void ldomDocumentWriterFilter::appendStyle( const lChar32 * style ) // return; // disabled lString32 oldStyle = node->getAttributeValue(attr_style); - if ( !oldStyle.empty() && oldStyle.at(oldStyle.length()-1)!=';' ) + if ( !oldStyle.empty() && oldStyle.lastChar()!=';' ) oldStyle << "; "; oldStyle << style; node->setAttributeValue(LXML_NS_NONE, attr_style, oldStyle.c_str()); diff --git a/crengine/src/mathml.cpp b/crengine/src/mathml.cpp index 91e13a9f9..b851da561 100644 --- a/crengine/src/mathml.cpp +++ b/crengine/src/mathml.cpp @@ -617,12 +617,13 @@ lString32 MathMLHelper::getMathMLAdjustedText(ldomNode * node, const lChar32 * t // (no substitution for "normal") if ( mathvariant != mathml_mathvariant_normal ) { len = mtext.length(); + lChar32 *ptr = mtext.modify(); for ( int i=0; i MODIFIER LETTER CIRCUMFLEX ACCENT + lChar32 *ptr = mtext.modify(); + switch (ptr[0]) { + case 0x0302: // COMBINING CIRCUMFLEX ACCENT + ptr[0] = 0x02C6; // => MODIFIER LETTER CIRCUMFLEX ACCENT break; - case 0x0303: // COMBINING TILDE - mtext[0] = 0x007E; // => TILDE + case 0x0303: // COMBINING TILDE + ptr[0] = 0x007E; // => TILDE break; - case 0x0304: // COMBINING MACRON - mtext[0] = 0x00AF; // => MACRON + case 0x0304: // COMBINING MACRON + ptr[0] = 0x00AF; // => MACRON break; - case 0x0305: // COMBINING OVERLINE - mtext[0] = 0x00AF; // => MACRON + case 0x0305: // COMBINING OVERLINE + ptr[0] = 0x00AF; // => MACRON break; - case 0x030C: // COMBINING CARON - mtext[0] = 0x02C7; // => CARON + case 0x030C: // COMBINING CARON + ptr[0] = 0x02C7; // => CARON break; - case 0x0332: // COMBINING LOW LINE - mtext[0] = 0x005F; // => LOW LINE + case 0x0332: // COMBINING LOW LINE + ptr[0] = 0x005F; // => LOW LINE break; default: break;