Derby
  1. Derby
  2. DERBY-212

Optimize some specific methods in Network Server to improve performance

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.1.1.0
    • Fix Version/s: 10.3.1.4
    • Component/s: Network Server
    • Labels:
      None
    • Bug behavior facts:
      Performance

      Description

      In reviewing the Network Server Code and profiling there were
      several areas that showed potential for providing performance
      improvement.

      Functions that need optimizing to prevent unneeded object
      creation and excessive decoding
      parsePKGNAMCSN()
      parseSQLDTA_work()
      buildDB2CursorName()

      In DDMWriter and DDMReader, use System Routines in
      java.util.Arrays and System.arrayCopy instead of
      writing code to do functions like copy arrays and
      pad strings.

      1. DERBY-212-parsePKGNAMCSN-2.diff
        39 kB
        Knut Anders Hatlen
      2. throughput-normalized.png
        4 kB
        Knut Anders Hatlen
      3. throughput.png
        4 kB
        Knut Anders Hatlen

        Activity

        Hide
        Dyre Tjeldvoll added a comment -

        All subtasks have been closed

        Show
        Dyre Tjeldvoll added a comment - All subtasks have been closed
        Hide
        Knut Anders Hatlen added a comment -

        Reopening the issue since my patch didn't address everything mentioned
        in the description. Dyre has started optimizing parseSQLDTA_work(), so
        I reassign the issue to him.

        Show
        Knut Anders Hatlen added a comment - Reopening the issue since my patch didn't address everything mentioned in the description. Dyre has started optimizing parseSQLDTA_work(), so I reassign the issue to him.
        Hide
        Knut Anders Hatlen added a comment -

        I have uploaded a new patch (DERBY-212-parsePKGNAMCSN-2.diff) where I
        have tried to address Bryan's review comments.

        % svn stat

        M java/drda/org/apache/derby/impl/drda/DDMWriter.java
        A java/drda/org/apache/derby/impl/drda/DRDAString.java
        A java/drda/org/apache/derby/impl/drda/Pkgnamcsn.java
        M java/drda/org/apache/derby/impl/drda/DRDAResultSet.java
        M java/drda/org/apache/derby/impl/drda/DRDAStatement.java
        A java/drda/org/apache/derby/impl/drda/ConsistencyToken.java
        M java/drda/org/apache/derby/impl/drda/Database.java
        M java/drda/org/apache/derby/impl/drda/DRDAConnThread.java
        M java/drda/org/apache/derby/impl/drda/DDMReader.java

        Changes from the previous patch:

        1) The autoTrim variable in DRDAString was removed. Instead,
        DDMReader.readString() has an extra argument, unpad, which
        specifies whether trailing padding characters should be removed.

        2) DRDAString was simplified:

        • Support for converting between different CcsidManagers (and
          caching of them) was removed.
        • A new byte array is generated each time the length of a string
          changes, removing the need for an internal length field.
        • setBytes() (and DDMReader.readString()) no longer returns a
          boolean value to indicate whether the string value has
          changed. Instead, DRDAString has a new method, wasModified(),
          which returns true if the last call to setBytes() actually
          modified the string.

        3) Variable names in DRDAConnThread, Pkgnamcsn and DRDAStatement
        were made more consistent (pkgsn instead of secnumber and
        sectionNumber).

        4) Pkgnamcsn.getPkgcnstknStr() was renamed to getPkgcnstkn() and now
        returns an object of type ConsistencyToken instead of a
        string. ConsistencyToken is a new class in this patch.

        5) StmtKey is not an inner class in Database. It was moved to
        Pkgnamcsn and renamed to StatementKey. This change made the
        implementation of the class simpler, and it removed the need for
        caching the previous statement key in the Database class.

        I have run derbyall successfully. Comments are welcome. Thanks!

        Show
        Knut Anders Hatlen added a comment - I have uploaded a new patch ( DERBY-212 -parsePKGNAMCSN-2.diff) where I have tried to address Bryan's review comments. % svn stat M java/drda/org/apache/derby/impl/drda/DDMWriter.java A java/drda/org/apache/derby/impl/drda/DRDAString.java A java/drda/org/apache/derby/impl/drda/Pkgnamcsn.java M java/drda/org/apache/derby/impl/drda/DRDAResultSet.java M java/drda/org/apache/derby/impl/drda/DRDAStatement.java A java/drda/org/apache/derby/impl/drda/ConsistencyToken.java M java/drda/org/apache/derby/impl/drda/Database.java M java/drda/org/apache/derby/impl/drda/DRDAConnThread.java M java/drda/org/apache/derby/impl/drda/DDMReader.java Changes from the previous patch: 1) The autoTrim variable in DRDAString was removed. Instead, DDMReader.readString() has an extra argument, unpad, which specifies whether trailing padding characters should be removed. 2) DRDAString was simplified: Support for converting between different CcsidManagers (and caching of them) was removed. A new byte array is generated each time the length of a string changes, removing the need for an internal length field. setBytes() (and DDMReader.readString()) no longer returns a boolean value to indicate whether the string value has changed. Instead, DRDAString has a new method, wasModified(), which returns true if the last call to setBytes() actually modified the string. 3) Variable names in DRDAConnThread, Pkgnamcsn and DRDAStatement were made more consistent (pkgsn instead of secnumber and sectionNumber). 4) Pkgnamcsn.getPkgcnstknStr() was renamed to getPkgcnstkn() and now returns an object of type ConsistencyToken instead of a string. ConsistencyToken is a new class in this patch. 5) StmtKey is not an inner class in Database. It was moved to Pkgnamcsn and renamed to StatementKey. This change made the implementation of the class simpler, and it removed the need for caching the previous statement key in the Database class. I have run derbyall successfully. Comments are welcome. Thanks!
        Hide
        Knut Anders Hatlen added a comment -

        I have uploaded a patch (DERBY-212-parsePKGNAMCSN.diff) which changes
        the following files:

        M java/drda/org/apache/derby/impl/drda/DDMWriter.java
        A java/drda/org/apache/derby/impl/drda/DRDAString.java
        A java/drda/org/apache/derby/impl/drda/Pkgnamcsn.java
        M java/drda/org/apache/derby/impl/drda/DRDAStatement.java
        M java/drda/org/apache/derby/impl/drda/Database.java
        M java/drda/org/apache/derby/impl/drda/DRDAConnThread.java
        M java/drda/org/apache/derby/impl/drda/DDMReader.java

        The patch only addresses the reported problems in
        parsePKGNAMCSN(). This method parses a PKGNAMCSN object and returns a
        string with all the fields concatenated. When other parts of the code
        need one of the fields, a StringTokenizer has to be used to fetch the
        field. This approach leads to a lot of unnecessary object allocation
        and string parsing.

        What the patch does is basically:

        1) Instead of returning a string from parsePKGNAMCSN(), an object of
        type Pkgnamcsn is returned. This object has accessor methods
        which are much cheaper than the methods in StringTokenizer.

        2) Since the strings read in parsePKGNAMCSN() often are repeated,
        reusing the strings seemed like a good idea. To achieve this, the
        class DRDAString was implemented. That class stores a byte array
        and can return the array as a string. Strings are cached to avoid
        allocation and decoding if the byte array has not
        changed. Pkgnamcsn objects are also cached, so a new one is only
        allocated when one of its fields has changed.

        3) In Database.java, a substring of the original PKGNAMCSN string
        was used as a hash key. With this patch, a new internal class
        StmtKey is added to Database. That class implements equals() and
        hashCode() in a way that enables its use as a hash key instead of
        the substring that was used originally. Caching is also
        implemented to avoid allocating new StmtKey objects when an old
        one can be reused.

        Derbyall ran without errors.

        I have also attached graphs showing the performance improvements. The
        graphs show the throughput of 1-100 clients running read-only load
        (select one row from a table which is in the page cache). The file
        throughput.png shows the throughput as transactions per second with
        and without the patch, and throughput-normalized.png shows the
        increase in percent for different number of clients. The graphs
        indicate a speedup of 5-8% with this particular load.

        Could someone please review and comment on the patch?

        Show
        Knut Anders Hatlen added a comment - I have uploaded a patch ( DERBY-212 -parsePKGNAMCSN.diff) which changes the following files: M java/drda/org/apache/derby/impl/drda/DDMWriter.java A java/drda/org/apache/derby/impl/drda/DRDAString.java A java/drda/org/apache/derby/impl/drda/Pkgnamcsn.java M java/drda/org/apache/derby/impl/drda/DRDAStatement.java M java/drda/org/apache/derby/impl/drda/Database.java M java/drda/org/apache/derby/impl/drda/DRDAConnThread.java M java/drda/org/apache/derby/impl/drda/DDMReader.java The patch only addresses the reported problems in parsePKGNAMCSN(). This method parses a PKGNAMCSN object and returns a string with all the fields concatenated. When other parts of the code need one of the fields, a StringTokenizer has to be used to fetch the field. This approach leads to a lot of unnecessary object allocation and string parsing. What the patch does is basically: 1) Instead of returning a string from parsePKGNAMCSN(), an object of type Pkgnamcsn is returned. This object has accessor methods which are much cheaper than the methods in StringTokenizer. 2) Since the strings read in parsePKGNAMCSN() often are repeated, reusing the strings seemed like a good idea. To achieve this, the class DRDAString was implemented. That class stores a byte array and can return the array as a string. Strings are cached to avoid allocation and decoding if the byte array has not changed. Pkgnamcsn objects are also cached, so a new one is only allocated when one of its fields has changed. 3) In Database.java, a substring of the original PKGNAMCSN string was used as a hash key. With this patch, a new internal class StmtKey is added to Database. That class implements equals() and hashCode() in a way that enables its use as a hash key instead of the substring that was used originally. Caching is also implemented to avoid allocating new StmtKey objects when an old one can be reused. Derbyall ran without errors. I have also attached graphs showing the performance improvements. The graphs show the throughput of 1-100 clients running read-only load (select one row from a table which is in the page cache). The file throughput.png shows the throughput as transactions per second with and without the patch, and throughput-normalized.png shows the increase in percent for different number of clients. The graphs indicate a speedup of 5-8% with this particular load. Could someone please review and comment on the patch?
        Hide
        Dyre Tjeldvoll added a comment -

        Added performance as a component

        Show
        Dyre Tjeldvoll added a comment - Added performance as a component
        Hide
        Dyre Tjeldvoll added a comment -

        Knut and I have looked at this, and Knut has patch for much of this that will be ready soon.

        Show
        Dyre Tjeldvoll added a comment - Knut and I have looked at this, and Knut has patch for much of this that will be ready soon.
        Hide
        Knut Anders Hatlen added a comment -

        The method buildDB2CursorName() mentioned in the description of this
        issue seems to have disappeared from the code. A comment in
        DRDAConnThread.parsePKGNAMCSN() says

        // Note: This string is parsed by DRDAStatement.buildDB2CursorName()

        but there is no such method in DRDAStatement. There is however a
        method called extractPkgcnstknStr() which parses the string returned
        by parsePKGNAMCSN() and seems to take a lot of resources.

        Show
        Knut Anders Hatlen added a comment - The method buildDB2CursorName() mentioned in the description of this issue seems to have disappeared from the code. A comment in DRDAConnThread.parsePKGNAMCSN() says // Note: This string is parsed by DRDAStatement.buildDB2CursorName() but there is no such method in DRDAStatement. There is however a method called extractPkgcnstknStr() which parses the string returned by parsePKGNAMCSN() and seems to take a lot of resources.

          People

          • Assignee:
            Dyre Tjeldvoll
            Reporter:
            Kathey Marsden
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development