[esnacc-dev] [PATCH] cxx-lib: do not use exceptions in performance critical operators

Kovács László laszlo.kovacs at neti.com
Tue Mar 21 14:32:29 UTC 2017


In asn-buf.cpp the operator<() and the operator==() methods
suffer from a performance issue, they are running in a loop until
an exception is not thrown. This is a very bad idea, and very
inefficient way of error handling. I have done some performance
tests with gproof. The performance has improved about 4 times better,
the code runs faster after my fix. I removed the exception handling from
operator<() and the operator==() and replaced them to a
boolean function, that can signal if an exception would be thrown.

Signed-off-by: László Kovács <laszlo.kovacs at neti.com> <sabbath24 at gmail.com>
Acked-by:      László Kovács <laszlo.kovacs at neti.com> <sabbath24 at gmail.com>
Tested-by:     László Kovács <laszlo.kovacs at neti.com> <sabbath24 at gmail.com>
Reported-by:   László Kovács <laszlo.kovacs at neti.com> <sabbath24 at gmail.com>

---

diff --git a/esnacc-ng-master/cxx-lib/inc/asn-buf.h
b/esnacc-ng-master/cxx-lib/inc/asn-buf.h
index 4d06590..4235fb9 100644
--- a/esnacc-ng-master/cxx-lib/inc/asn-buf.h
+++ b/esnacc-ng-master/cxx-lib/inc/asn-buf.h
@@ -32,13 +32,13 @@
 #define SNACCDLL_API __declspec(dllexport)
 #else
 #ifdef SNACCDLL_NONE
-#define SNACCDLL_API
+#define SNACCDLL_API
 #else
 #define SNACCDLL_API __declspec(dllimport)
 #endif
 #endif
 #else
-#define SNACCDLL_API
+#define SNACCDLL_API
 #endif
 #endif
 
@@ -115,7 +115,9 @@ public:
     void skip(size_t skipBytes);
     char PeekByte() const;
     char GetByte() const;
+    bool GetByteNoEx(char& ret) const;
     unsigned char GetUByte() const        { return (unsigned
char)GetByte();}
+    bool GetUByteNoEx(unsigned char &ret) const        { char val; bool
tmp=GetByteNoEx(val); ret=(unsigned char)val; return tmp; }
 //    unsigned long GetSeg(char* seg, long segLen) const;
     void GetSeg(char* seg, long segLen) const;
     void GetUSeg(unsigned char* seg, long segLen) const {
@@ -135,7 +137,7 @@ public:
     void insert(const AsnBuf& that);
     long splice(AsnBuf& b);
     void hexDump(std::ostream& os) const;
-  
+
 #ifdef _DEBUG
     void status(std::ostream& os);
 #endif
@@ -152,7 +154,7 @@ protected:
     mutable SNACC::Deck m_deck;
 };
 
-class SNACCDLL_API AsnBufBits
+class SNACCDLL_API AsnBufBits
 {
 private:
     std::streambuf* m_pbuf;                // Stream buffer containing
the bits
@@ -190,13 +192,13 @@ public:
         m_ulNumBits         = 0;
         m_ulBitsLeft        = 0;
     }
-   
+
     ~AsnBufBits(){ if (m_isInternalBuf && m_pbuf) delete m_pbuf; }
 
-   
+
     bool IsAligned()            { return bAlign; }
 
-   
+
 /*
     AsnBufBits & operator=(const AsnBufBits &buf)
     {
@@ -222,7 +224,7 @@ public:
     unsigned long GetBits(AsnBits& bits, unsigned long numBits);
     bool GetBit();
     unsigned char GetByte();
-   
+
     unsigned long  length(){ return m_ulBitsLeft; }
     int            OctetAlignWrite();
     int            OctetAlignRead();
@@ -230,12 +232,12 @@ public:
     void          hexDump(std::ostream &os);
     void          AppendTo(AsnBufBits &bufBitsOut);
 
-    AsnBufBits(const AsnBufBits& buf);   
+    AsnBufBits(const AsnBufBits& buf);
     AsnBufBits& operator=(const AsnBufBits& buf);
 
 private:
     unsigned char ReadByte();    // Reads next byte from the stream
-       
+
 };
 
 
@@ -254,14 +256,14 @@ public:
 
    const char *str()  { return m_pStart; }
    int         pcount() { return ( (m_buf + m_segSize) - m_pStart ); }
-
+
    long max_size() const {return m_segSize;}
 
    friend class AsnBuf;
 
 protected:
    virtual int_type        underflow();
-   virtual int_type        overflow(int c = EOF);  
+   virtual int_type        overflow(int c = EOF);
    virtual std::streamsize xsputn(const char *s, std::streamsize n);
     virtual std::streambuf::pos_type       
seekoff(std::streambuf::off_type off, std::ios_base::seekdir way,
                                    std::ios_base::openmode which =
std::ios_base::in | std::ios_base::out);
@@ -316,7 +318,7 @@ private:
 //
 
 // Deck contains a streambuf * and a bool flag that indicates
-// whether or not the streambuf can be deleted.
+// whether or not the streambuf can be deleted.
 //
 enum AsnBufType { FILE_TYPE=0, RVS_BUF_TYPE, IN_MEM_TYPE, EXT_MEM_TYPE } ;
 
@@ -329,7 +331,7 @@ public:
       m_cDataW = 0x00;
       m_iBitPosW = 0;
       m_iBitPosR = 0;
-      first = NULL;
+      first = NULL;
       second = EXT_MEM_TYPE;
    }
 #endif
@@ -339,8 +341,8 @@ public:
       m_cDataW = 0x00;
       m_iBitPosW = 0;
       m_iBitPosR = 0;
-      first = sb;
-      second = EXT_MEM_TYPE;
+      first = sb;
+      second = EXT_MEM_TYPE;
    }
 
    Card(const std::stringstream &ss)
@@ -348,7 +350,7 @@ public:
       m_cDataW = 0x00;
       m_iBitPosW = 0;
       m_iBitPosR = 0;
-      first = ss.rdbuf();
+      first = ss.rdbuf();
       second = EXT_MEM_TYPE;
    }
 
@@ -357,16 +359,16 @@ public:
       m_cDataW = 0x00;
       m_iBitPosW = 0;
       m_iBitPosR = 0;
-      first = pRvsBuf;
+      first = pRvsBuf;
       second = RVS_BUF_TYPE;
    }
- 
+
    Card(AsnFileSeg *pFs)
    {  m_cDataR = 0x00;
       m_cDataW = 0x00;
       m_iBitPosW = 0;
       m_iBitPosR = 0;
-      first = pFs;
+      first = pFs;
       second = FILE_TYPE;
    }
 
@@ -378,7 +380,7 @@ public:
       first = o.first;
       second = o.second;
    }
-  
+
    virtual ~Card()
    {
       if (second != EXT_MEM_TYPE)
@@ -386,14 +388,14 @@ public:
    }
 
    int        BitPosW(){return m_iBitPosW;}
-   int      SetBitPosW(int newBitPos){m_iBitPosW = newBitPos; return
m_iBitPosW;}  
-  
+   int      SetBitPosW(int newBitPos){m_iBitPosW = newBitPos; return
m_iBitPosW;}
+
    int        BitPosR(){return m_iBitPosR;}
    int        SetBitPosR(int newBitPos){m_iBitPosR = newBitPos; return
m_iBitPosR;}
 
    char     cDataW(){return m_cDataW;}
    char     cDataW(char chr){m_cDataW=chr; return m_cDataW;}
-  
+
    char     cDataR(){return m_cDataR;}
    char     cDataR(char chr){m_cDataR=chr; return m_cDataR;}
 
diff --git a/esnacc-ng-master/cxx-lib/src/asn-buf.cpp
b/esnacc-ng-master/cxx-lib/src/asn-buf.cpp
index 62d61df..defa019 100644
--- a/esnacc-ng-master/cxx-lib/src/asn-buf.cpp
+++ b/esnacc-ng-master/cxx-lib/src/asn-buf.cpp
@@ -38,8 +38,8 @@ AsnBuf::AsnBuf()
 }
 
 AsnBuf::AsnBuf(const char *seg, size_t segLen)
-{
-   m_card = m_deck.insert(m_deck.begin(), new Card(new AsnRvsBuf(seg,
segLen)));
+{
+   m_card = m_deck.insert(m_deck.begin(), new Card(new AsnRvsBuf(seg,
segLen)));
 }
 
 AsnBuf::AsnBuf(const std::stringstream &ss)
@@ -126,7 +126,7 @@ void AsnBuf::PutSegRvs(const char *seg, size_t segLen)
       if (segLen > 0)
       {
          // reuse any existing card(s)
-         if (m_card == m_deck.begin())
+         if (m_card == m_deck.begin())
             m_card = m_deck.insert(m_deck.begin(), new Card(new
AsnRvsBuf));
          else
             --m_card;
@@ -163,7 +163,7 @@ void AsnBuf::ResetMode(std::ios_base::openmode mode)
const
 
    if (mode == std::ios_base::in)
       m_card = m_deck.begin();
-   else
+   else
    {
       m_card = m_deck.end();
       if ( ! m_deck.empty())
@@ -222,11 +222,11 @@ void AsnBuf::GetSeg(char *seg, long segLen) const
     FUNC("AsnBuf::GetSeg()");
    long bytesRead = 0;
    long lTmp;
-  
+
    while (segLen > 0 && m_card != m_deck.end() )
    {
       lTmp = (*m_card)->rdbuf()->sgetn(&seg[bytesRead], segLen);
-  
+
       bytesRead += lTmp;
       if (lTmp != segLen)
          m_card++;
@@ -263,6 +263,27 @@ void AsnBuf::GetSeg(std::string &seg, long segLen)
const
    }
 }
 
+bool AsnBuf::GetByteNoEx(char& ret) const
+{
+    std::streambuf::int_type ch;
+
+    if(m_card==m_deck.end()) return false;
+
+    do
+    {
+        ch=(*m_card)->rdbuf()->sbumpc();
+        if(ch==EOF)
+        {
+            ++m_card;
+            if(m_card==m_deck.end()) return false;
+        }
+    }
+    while(ch==EOF);
+
+    ret=(char)ch;
+    return true;
+}
+
 bool AsnBuf::operator<(const AsnBuf &rhs) const
 {
    bool lessThan = true;
@@ -271,32 +292,23 @@ bool AsnBuf::operator<(const AsnBuf &rhs) const
    rhs.ResetMode();
    std::streambuf::int_type ch1;
    std::streambuf::int_type ch2;
+   unsigned char Ch1;
+   unsigned char Ch2;
+
    while ( lessThan )
    {
-      try
-      {
-         ch1 = GetUByte();
-      }
-      catch (BufferException &)
-      {
-         ch1 = EOF;
-      }
+      if(GetUByteNoEx(Ch1)) ch1=Ch1;
+      else ch1=EOF;
 
-      try
-      {
-         ch2 = rhs.GetUByte();
-      }
-      catch (BufferException &)
-      {
-         ch2 = EOF;
-      }
+      if(rhs.GetUByteNoEx(Ch2)) ch2=Ch2;
+      else ch2=EOF;
 
       if ((ch1 == EOF) && (ch2 == EOF))
       {
          if (firstTime)
             lessThan = false;
          break;
-      }
+      }
       else if (ch2 == EOF)
       {
          lessThan = false;
@@ -313,7 +325,9 @@ bool AsnBuf::operator<(const AsnBuf &rhs) const
          break;
 
       firstTime = false;
+
    }
+
    ResetMode();
    rhs.ResetMode();
    return lessThan;
@@ -344,10 +358,10 @@ unsigned long AsnBuf::length() const
          endPos = (*tmpCard)->rdbuf()->pubseekoff(0,
std::ios_base::end, std::ios_base::in);
          (*tmpCard)->rdbuf()->pubseekpos(currPos, std::ios_base::in);
          bytesRemaining += endPos - currPos;
-           
+
          tmpCard++;
       }
-  
+
       //SetReadLoc(readLoc);
    }
 
@@ -366,8 +380,8 @@ void AsnBuf::UnGetBytes(long lBytesToPutBack) const
         if( ((*m_card)->rdbuf()->sungetc()) == EOF )
         {
             if( m_card != m_deck.begin() )
-            {   
-                m_card--;               
+            {
+                m_card--;
             }
             else
             {
@@ -376,7 +390,7 @@ void AsnBuf::UnGetBytes(long lBytesToPutBack) const
         }
         else
         {
-            lBytesToPutBack--;   
+            lBytesToPutBack--;
         }
     }
 }
@@ -420,14 +434,14 @@ void AsnBuf::GrabAny(AsnBuf &anyBuf, AsnLen
&bytesDecoded) const
    AsnLen tmpLen = bytesDecoded;
    AsnBufLoc readLoc = GetReadLoc();
    AsnLen lTmpbytesDecoded=0;
-  
+
    // Decode tag of the ANY.  This will be encoded into AnyBuf after
the length
    //
    // @todo: decode the tag using an ASN.1 TagId structure, and check
    // validity
    //
    (void)BDecTag(*this, bytesDecoded);
-  
+
    // Decode length of the ANY.  This will be encoded into anyBuf after
the data.
    len = BDecLen(*this, bytesDecoded);
 
@@ -447,10 +461,10 @@ void AsnBuf::GrabAny(AsnBuf &anyBuf, AsnLen
&bytesDecoded) const
    {
       throw BufferException("len error from BDecLen call", STACK_ENTRY);
    }
-  
+
    SetReadLoc(readLoc);
-  
-   // length is greater than the magic size and
+
+   // length is greater than the magic size and
    // the m_card contains a file then store a
    // AsnFileSeg object in output buf.
    //
@@ -514,7 +528,7 @@ AsnBufLoc AsnBuf::GetReadLoc() const
 #ifdef _DEBUG
    Card *pCard=*m_card;
 #endif
-   if (bl.m_offset == -1)
+   if (bl.m_offset == -1)
        bl.m_offset = 0;
    return bl;
 }
@@ -524,11 +538,11 @@ void AsnBuf::SetReadLoc(const AsnBufLoc &bl) const
    FUNC("AsnBuf::setReadLoc()");
 
    Deck::iterator i = m_deck.begin();
-  
+
    // first make sure interator bl.m_card is between current card
    // start of the deck.
    //
-  
+
    while (i != bl.m_card && i != m_deck.end())
    {
       i++;
@@ -607,7 +621,7 @@ void BDEC_2ND_EOC_OCTET(const SNACC::AsnBuf &b,
SNACC::AsnLen &bytesDecoded)
 
    if ((b.GetByte() != 0))
       throw EXCEPT("second octet of EOC not zero", DECODE_ERROR);
-  
+
    bytesDecoded++;
 }
 
@@ -647,16 +661,16 @@ bool AsnBuf::operator == (const AsnBuf &b) const
    b.ResetMode();
    std::streambuf::int_type ch1;
    std::streambuf::int_type ch2;
+   unsigned char Ch1;
+   unsigned char Ch2;
+
    while ( equal )
    {
-      try
-      {
-         ch1 = GetUByte();
-      }
-      catch (BufferException &)
-      {
-         ch1 = EOF;
-      }
+      if(GetUByteNoEx(Ch1)) ch1=Ch1;
+      else ch1=EOF;
+
+      if(b.GetUByteNoEx(Ch2)) ch2=Ch2;
+      else ch2=EOF;
 
       try
       {
@@ -682,14 +696,14 @@ void AsnBuf::hexDump(std::ostream &os) const
 {
    bool done = false;
    int ch;
-  
+
    ResetMode();
 
    std::hex(os);
 
    while (! done)
    {
-     
+
      try
       {
          ch = GetUByte();
@@ -704,16 +718,16 @@ void AsnBuf::hexDump(std::ostream &os) const
          os.unsetf(std::ios_base::hex);
          done = true;
       }
-
+
    }
 
- 
+
 }
 
 
 
 std::ostream & operator<<(std::ostream &os, const SNACC::AsnBuf &b)
-{
+{
     SNACC::Deck::const_iterator card = b.deck().begin();
     while (card != b.deck().end())
     {
@@ -722,7 +736,7 @@ std::ostream & operator<<(std::ostream &os, const
SNACC::AsnBuf &b)
        card++;
     }
     os.flush();
-    return os;
+    return os;
 }
 
 long Card::size()
@@ -730,15 +744,15 @@ long Card::size()
   long currPos, endPos;
 
   currPos = first->pubseekoff(0, std::ios_base::cur, std::ios_base::in);
- 
+
   endPos = first->pubseekoff(0, std::ios_base::end, std::ios_base::in);
-
+
   if (currPos != -1)
       first->pubseekpos(currPos, std::ios_base::in);
-
+
   if (endPos == -1)
      endPos = 0;
-    
+
   return endPos;
 }
 
@@ -747,14 +761,14 @@ long Card::length()
   long currPos, endPos;
 
   currPos = first->pubseekoff(0, std::ios_base::cur, std::ios_base::in);
- 
+
   if (currPos == -1)
      currPos = 0;
- 
+
   endPos = first->pubseekoff(0, std::ios_base::end, std::ios_base::in);
-
+
   first->pubseekpos(currPos, std::ios_base::in);
-
+
   if (endPos == -1)
      endPos = 0;
 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.esnacc.org/pipermail/dev/attachments/20170321/c8ef3969/attachment-0001.html>


More information about the dev mailing list