castor
  1. castor
  2. CASTOR-1085

TransactionContext suffers from a crappy implementation. ;)

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.9.6
    • Fix Version/s: 0.9.9 M1
    • Component/s: JDO
    • Labels:
      None
    • Number of attachments :
      6

      Description

      TransactionManager's internal data structures, and procedures for execution, are based on serially and repeating walking lists of objects in order retrieve even trivial information...

      This is a brand-new, from-scratch patch to the problem that tries to localise it to only TransactionManager API changes which can be done against head.

      It has the following properties:

      • Rips code out of commons-collections so that we have an IdentityHashMap on pre-1.4 systems; this is within the legal use, and it's in a place that's obvious and follows Sun guidelines (org.exolab.castor.org.apache...)
      • Re-implements all of our state tracking in an ObjectTracker that relies on IdentityHashMap to track object-to-data information without using the classes' own hashCode() and equals() calls, which may not actually be accurate.
      • Cleans up some code duplication as a part of the effort to minimise the amount of code in the class; now down to <2000 lines of code.

      Performance results of new patch, in milliseconds, using the bug_rop patch:

      10K Objects via the ROP patch:

      <pre>
      PRE-PATCH:
      17 Apr 2005 16:41:05.218 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:41:31.897 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadWriteEmpty 29 829 17,076 8,744 0
      17 Apr 2005 16:41:31.899 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:41:56.365 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadWriteCached 0 417 15,315 8,734 0
      17 Apr 2005 16:41:56.367 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:42:43.494 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadWriteOidEmpty 19 104 38,132 8,871 0
      17 Apr 2005 16:42:43.496 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:43:16.196 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadWriteOidCached 0 109 24,134 8,456 0
      17 Apr 2005 16:43:16.197 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:43:20.322 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadOnlyEmpty 10 394 3,719 1 0
      17 Apr 2005 16:43:20.324 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:43:24.421 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadOnlyCached 0 257 3,838 1 0
      17 Apr 2005 16:43:24.422 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:43:37.478 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadOnlyOidEmpty 9 116 12,930 1 0
      17 Apr 2005 16:43:37.479 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:43:38.157 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadOnlyOidCached 0 95 581 1 0
      17 Apr 2005 16:43:38.159 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:43:38.575 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadOnlyOidOnly 0 105 311 0 0

      POST-PATCH:
      17 Apr 2005 16:34:17.446 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:34:23.698 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadWriteEmpty 111 619 5,213 307 0
      17 Apr 2005 16:34:23.700 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:34:28.546 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadWriteCached 0 420 4,282 143 0
      17 Apr 2005 16:34:28.547 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:34:42.718 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadWriteOidEmpty 17 110 13,897 147 0
      17 Apr 2005 16:34:42.719 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:34:43.923 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadWriteOidCached 0 106 960 138 0
      17 Apr 2005 16:34:43.924 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:34:48.809 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadOnlyEmpty 12 460 4,411 2 0
      17 Apr 2005 16:34:48.811 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:34:53.436 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadOnlyCached 0 254 4,368 2 0
      17 Apr 2005 16:34:53.438 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:35:07.358 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadOnlyOidEmpty 9 104 13,803 4 0
      17 Apr 2005 16:35:07.360 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:35:08.252 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadOnlyOidCached 0 114 772 5 0
      17 Apr 2005 16:35:08.260 [INFO] [main] [jdo.bug_rop.TestLoadService] - begin result iterate commit close
      17 Apr 2005 16:35:08.920 [INFO] [main] [jdo.bug_rop.TestLoadService] - ReadOnlyOidOnly 0 105 551 3 0
      </pre>

      Now, there's likely several state checks that I can optimize further, but I'll worry about that in future updates, post-commit - for now, this is 3x faster during iterate for the standard read write tests, and eliminates the commit time completely on the majority of the most offensive tests.

      This needs testing, folks; but all tests that I ran against it from the test suite pass. It'll go into my codebase for ship this weekend, and will likely ship on my live service sometime next week, if things go well.

      1. patch.20050610.txt
        195 kB
        Werner Guttmann
      2. patch.20050620-02.txt
        309 kB
        Werner Guttmann
      3. patch-C1085-20050618.txt
        195 kB
        Ralf Joachim
      4. patch-C1085-20050619.txt
        190 kB
        Ralf Joachim
      5. patch-C1085-20050620.txt
        46 kB
        Ralf Joachim
      6. TransactionContext-Rewrite.diff
        820 kB
        Gregory Block

        Issue Links

          Activity

          Hide
          Ralf Joachim added a comment -

          I forgot to mention that I also removed engineToObject and molderToObject mappings from ObjectTracker with my last patch as they are not needed ATM.

          Show
          Ralf Joachim added a comment - I forgot to mention that I also removed engineToObject and molderToObject mappings from ObjectTracker with my last patch as they are not needed ATM.
          Hide
          Werner Guttmann added a comment -

          Ralf , there's a thread in the user mailing list archive that discusses the use of JDK vesions. Afair, it was commonly agreed that as of 0.9.9, we don't support JDK 1.3 anymore. As such, one could consider switching to the java.util classes for this patch.

          Show
          Werner Guttmann added a comment - Ralf , there's a thread in the user mailing list archive that discusses the use of JDK vesions. Afair, it was commonly agreed that as of 0.9.9, we don't support JDK 1.3 anymore. As such, one could consider switching to the java.util classes for this patch.
          Hide
          Ralf Joachim added a comment -

          Finished tasks:

          1. Add test cases for IdentitySet.toArray() and IdentitySet.toArray(Object[]).
          2. Comment and codestyle IdentityMap and IdentitySet.

          Patch only contains changed classes from src/main/org.castor.util and src/tests/utf.org.castor.util

          Show
          Ralf Joachim added a comment - Finished tasks: 1. Add test cases for IdentitySet.toArray() and IdentitySet.toArray(Object[]). 2. Comment and codestyle IdentityMap and IdentitySet. Patch only contains changed classes from src/main/org.castor.util and src/tests/utf.org.castor.util
          Hide
          Werner Guttmann added a comment -

          Updated patch, with tasks 3), 4) and 6) finished.

          Show
          Werner Guttmann added a comment - Updated patch, with tasks 3), 4) and 6) finished.
          Hide
          Werner Guttmann added a comment -

          Patch committed as is ...

          Show
          Werner Guttmann added a comment - Patch committed as is ...

            People

            • Assignee:
              Gregory Block
              Reporter:
              Gregory Block
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 1 hour, 30 minutes
                1h 30m