Skip to content

Commit

Permalink
Avoid unnecessary string modifications (#543)
Browse files Browse the repository at this point in the history 
Details at #543:
- lvstring: drop `& operator []` accessors
  As they end up always being used, even for read-only lookup,
  triggering unnecessary calls to `modify()` strings.
- crengine: avoid unnecessary use of `lStringXX:at(…)`
  Which could trigger a `modify()`.
- lvstring: fix `lastChar/firstChar` implementations
  Don't use `at()`, which could trigger a call to `modify()`.
- lvstring: drop `at()` methods
  Not used anymore.
  • Loading branch information
@benoit-pierre
benoit-pierre authored Nov 1, 2023
1 parent 156d095 commit 22191fd
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 31 deletions.
16 changes: 4 additions & 12 deletions crengine/include/lvstring.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down Expand Up @@ -815,18 +811,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
Expand Down
8 changes: 4 additions & 4 deletions crengine/src/lvrend.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down Expand Up @@ -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 );
}
Expand Down Expand Up @@ -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 );
}
Expand Down Expand Up @@ -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 );
}
Expand Down
2 changes: 1 addition & 1 deletion crengine/src/lvtinydom.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
30 changes: 16 additions & 14 deletions crengine/src/mathml.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<len; i++ ) {
lChar32 c = substitute_codepoint(mtext[i], mathvariant);
// If c==0, no available substitution : we avoid modifying
// the lString32 when not needed (as it may malloc/free)
if (c != 0) {
mtext[i] = c;
ptr[i] = c;
}
}
}
Expand All @@ -648,24 +649,25 @@ lString32 MathMLHelper::getMathMLAdjustedText(ldomNode * node, const lChar32 * t
// as we meet them.
// Some small list at https://trac.webkit.org/changeset/203714/webkit
// More at https://lists.w3.org/Archives/Public/www-math/2018Nov/0005.html
switch (mtext[0]) {
case 0x0302: // COMBINING CIRCUMFLEX ACCENT
mtext[0] = 0x02C6; // => 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;
Expand Down

0 comments on commit 22191fd

Please sign in to comment.