Derby
  1. Derby
  2. DERBY-5472

Speed up MemoryLeakFixesTest.testRepeatedDatabaseCreationWithAutoStats()

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.9.1.0
    • Fix Version/s: 10.9.1.0
    • Component/s: Test
    • Labels:
      None

      Description

      MemoryLeakFixesTest.testRepeatedDatabaseCreationWithAutoStats() takes fairly long time. When I run it with -Xmx16M (per instructions in the comments) in my environment, that test case alone takes 80-90 seconds. And it runs twice (embedded and client) so it takes nearly 3 minutes in total.

      There are ways to speed it up and still have it expose DERBY-5336, for which it was originally written:

      1) The body of the test is executed 50 times. When the fix for DERBY-5336 is backed out, the test case typically fails in the 8th or 9th iteration, so 20 iterations should be enough.

      2) In each iteration, a table with 500 rows is created. Since the goal is to get the istat daemon to run, we only need to insert enough rows to exceed derby.storage.indexStats.debug.createThreshold, which is 100 by default. Reducing the size to for example 200 rows would be OK.

      3) When populating the table, a select statement is compiled and executed in between each insert statement. To get the istat daemon running, it's sufficient to execute a single select statement once the table is populated.

      4) Every insert statement is compiled separately. Would be better to compile once and execute multiple times.

      5) Populating the table could happen with auto-commit off.

      1. d5472-phoneme.diff
        1 kB
        Knut Anders Hatlen
      2. d5472.diff
        4 kB
        Knut Anders Hatlen

        Issue Links

          Activity

          Knut Anders Hatlen created issue -
          Knut Anders Hatlen made changes -
          Field Original Value New Value
          Status Open [ 1 ] In Progress [ 3 ]
          Hide
          Knut Anders Hatlen added a comment -

          The attached patch makes the suggested changes to the test case. I've verified that it still fails with OOME when backing out DERBY-5336 and running with -Xmx16M.

          This reduces the time to run MemoryLeakFixesTest from 210 seconds to 40 seconds in my environment.

          Show
          Knut Anders Hatlen added a comment - The attached patch makes the suggested changes to the test case. I've verified that it still fails with OOME when backing out DERBY-5336 and running with -Xmx16M. This reduces the time to run MemoryLeakFixesTest from 210 seconds to 40 seconds in my environment.
          Knut Anders Hatlen made changes -
          Attachment d5472.diff [ 12499682 ]
          Knut Anders Hatlen made changes -
          Link This issue relates to DERBY-5336 [ DERBY-5336 ]
          Knut Anders Hatlen made changes -
          Issue & fix info Patch Available [ 10102 ]
          Hide
          Bryan Pendleton added a comment -

          Did you happen to benchmark the effects of the 5 changes individually? Or do you only have a number for the effect of all 5 altogether?

          I think the speedup is great; making tests run faster means we can run the tests more often and in more configurations.

          I was just wondering if you knew which change had what effect on the overall runtime.

          Show
          Bryan Pendleton added a comment - Did you happen to benchmark the effects of the 5 changes individually? Or do you only have a number for the effect of all 5 altogether? I think the speedup is great; making tests run faster means we can run the tests more often and in more configurations. I was just wondering if you knew which change had what effect on the overall runtime.
          Hide
          Knut Anders Hatlen added a comment -

          Hi Bryan,

          I didn't benchmark them individually, but here's approximately what I saw as I added them:

          With no changes: 85 sec for the embedded case

          2+3+4 reduced it to 10 sec

          1+2+3+4 reduced it to 5 sec

          1+2+3+4+5 reduced it to 4.5 sec (don't know if the difference between 5 and 4.5 is significant or just variation...)

          So it's difficult to say if the improvements are mostly because of the reduced number of iterations, or if the reduced compilation contributed. Disabling auto-commit didn't seem to do much, but I ran the test on a file system where synchronous writes had been disabled, so the cost of the extra commits cannot really be seen from these experiments.

          I also had a hidden agenda, as I hope that removing the excessive recompilation will allow us to remove the work-around for DERBY-5412 in the test case, but I haven't verified that yet.

          But now you made me curious, so I guess I'll have to run the experiments to see how costly the compilation was.

          If it turns out compiling insert statements actually contributes to the time it takes to run the test, OrderByAndSortAvoidanceTest would be the next test to look at. That test has thousands of hard-coded insert statements in the decorator, each of them needs to be compiled separately.

          Show
          Knut Anders Hatlen added a comment - Hi Bryan, I didn't benchmark them individually, but here's approximately what I saw as I added them: With no changes: 85 sec for the embedded case 2+3+4 reduced it to 10 sec 1+2+3+4 reduced it to 5 sec 1+2+3+4+5 reduced it to 4.5 sec (don't know if the difference between 5 and 4.5 is significant or just variation...) So it's difficult to say if the improvements are mostly because of the reduced number of iterations, or if the reduced compilation contributed. Disabling auto-commit didn't seem to do much, but I ran the test on a file system where synchronous writes had been disabled, so the cost of the extra commits cannot really be seen from these experiments. I also had a hidden agenda, as I hope that removing the excessive recompilation will allow us to remove the work-around for DERBY-5412 in the test case, but I haven't verified that yet. But now you made me curious, so I guess I'll have to run the experiments to see how costly the compilation was. If it turns out compiling insert statements actually contributes to the time it takes to run the test, OrderByAndSortAvoidanceTest would be the next test to look at. That test has thousands of hard-coded insert statements in the decorator, each of them needs to be compiled separately.
          Hide
          Knut Anders Hatlen added a comment -

          More numbers:

          Only 4 (compiling the INSERT statement once and execute multiple times) reduced the time from 85 sec to 58 sec.

          Doing 3 (compile/execute the SELECT statement only once) in addition to 4, reduced the time further down to 12 sec.

          So it looks like reducing the amount of compilation is actually more important than reducing the number of iterations.

          Show
          Knut Anders Hatlen added a comment - More numbers: Only 4 (compiling the INSERT statement once and execute multiple times) reduced the time from 85 sec to 58 sec. Doing 3 (compile/execute the SELECT statement only once) in addition to 4, reduced the time further down to 12 sec. So it looks like reducing the amount of compilation is actually more important than reducing the number of iterations.
          Hide
          Bryan Pendleton added a comment -

          > it looks like reducing the amount of compilation is actually more important

          An interesting finding!

          Thank you for taking the extra time to break down the numbers and share your observations.

          Show
          Bryan Pendleton added a comment - > it looks like reducing the amount of compilation is actually more important An interesting finding! Thank you for taking the extra time to break down the numbers and share your observations.
          Hide
          Knut Anders Hatlen added a comment -

          Committed revision 1186630.

          Show
          Knut Anders Hatlen added a comment - Committed revision 1186630.
          Knut Anders Hatlen made changes -
          Fix Version/s 10.9.0.0 [ 12316344 ]
          Issue & fix info Patch Available [ 10102 ]
          Priority Major [ 3 ] Minor [ 4 ]
          Hide
          Knut Anders Hatlen added a comment -

          I've now verified that the changes in this issue makes the workaround for DERBY-5412 unnecessary. The VM limit on the number of class names in phoneME is no longer exceeded. The attached patch removes the workaround.

          Show
          Knut Anders Hatlen added a comment - I've now verified that the changes in this issue makes the workaround for DERBY-5412 unnecessary. The VM limit on the number of class names in phoneME is no longer exceeded. The attached patch removes the workaround.
          Knut Anders Hatlen made changes -
          Attachment d5472-phoneme.diff [ 12499826 ]
          Hide
          Knut Anders Hatlen added a comment -

          Committed revision 1186656.

          Closing the issue.

          Show
          Knut Anders Hatlen added a comment - Committed revision 1186656. Closing the issue.
          Knut Anders Hatlen made changes -
          Status In Progress [ 3 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Gavin made changes -
          Workflow jira [ 12638363 ] Default workflow, editable Closed status [ 12796666 ]

            People

            • Assignee:
              Knut Anders Hatlen
              Reporter:
              Knut Anders Hatlen
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development