Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-beta2
    • Fix Version/s: 2.0.0
    • Component/s: performance
    • Labels:
      None

      Description

      There have been a number of mailing list posts where the net of the post is that someone doesn't want OpenJPA to detach their Entities because after the commit happens, then are all going to be gc'd. All of the detach processing ends up being a waste. I also observed this same behavior when running some performance tests.

      With this JIRA I'm going to add a new openjpa.DetachState configuration option which will enable a new, super fast, minimalistic detach approach. This change is targeted at detach processing that happens due to an AutoDetach. Explicit detaches(ie: em.detach(..) ) will work as they always have before.

        Issue Links

          Activity

          Hide
          Rick Curtis added a comment -

          Attaching a patch with a bulk of the detach changes. There is still some work to be done, but I wanted to get some input before going too far along with this.

          Show
          Rick Curtis added a comment - Attaching a patch with a bulk of the detach changes. There is still some work to be done, but I wanted to get some input before going too far along with this.
          Hide
          Rick Curtis added a comment -

          Committed code changes, a simple UT, and doc to trunk. At this point I have a very basic unit test showing that I didn't break anything major.

          Show
          Rick Curtis added a comment - Committed code changes, a simple UT, and doc to trunk. At this point I have a very basic unit test showing that I didn't break anything major.
          Hide
          Rick Curtis added a comment -

          Committed code changes, doc, and a simple UT to trunk.

          Additional unit tests still need to be created.

          Show
          Rick Curtis added a comment - Committed code changes, doc, and a simple UT to trunk. Additional unit tests still need to be created.
          Hide
          Rick Curtis added a comment -

          Ideally this detach area could use some refactoring. Hopefully I'll get back to this next week.

          Show
          Rick Curtis added a comment - Ideally this detach area could use some refactoring. Hopefully I'll get back to this next week.
          Hide
          Donald Woods added a comment -

          code is being checked into 2.0.0, so updating affects/fix version to reflect that

          Show
          Donald Woods added a comment - code is being checked into 2.0.0, so updating affects/fix version to reflect that
          Hide
          Patrick Linskey added a comment -

          At first glance, I'd expect the 'loaded' detach mode to be pretty optimal – what is it doing that's causing headaches?

          Show
          Patrick Linskey added a comment - At first glance, I'd expect the 'loaded' detach mode to be pretty optimal – what is it doing that's causing headaches?
          Hide
          Pinaki Poddar added a comment -

          Revision 918634 has added new version field to persistent entities for detach tests.
          What are the assumptions made in the new detach technique that required these entities be added with a version field? If an entity does not have a version field how will be its impact?

          Show
          Pinaki Poddar added a comment - Revision 918634 has added new version field to persistent entities for detach tests. What are the assumptions made in the new detach technique that required these entities be added with a version field? If an entity does not have a version field how will be its impact?
          Hide
          Rick Curtis added a comment -

          @Pinaki - Per the doc that was committed in 918643: "...It is highly recommended that all Entities have a @Version field when using this property."

          On second thought, I probably need to make having a version field a requirement. The version field is used when we calculate whether or not a given Entity is detached when there is no state manager present.

          @Patrick - Sorry I missed your update before. From a high level this change can be described as, "OpenJPA give me my POJOs back." We found that the loaded setting does a fair amount of work validating that fields that should be loaded actually are. I short cut that entire path and completed the minimum amount of work to disassociate an Entity from the PersistenceContext.

          http://n2.nabble.com/Extreme-performance-problems-with-OpenJPA-commit-td4497009.html

          Show
          Rick Curtis added a comment - @Pinaki - Per the doc that was committed in 918643: "...It is highly recommended that all Entities have a @Version field when using this property." On second thought, I probably need to make having a version field a requirement. The version field is used when we calculate whether or not a given Entity is detached when there is no state manager present. @Patrick - Sorry I missed your update before. From a high level this change can be described as, "OpenJPA give me my POJOs back." We found that the loaded setting does a fair amount of work validating that fields that should be loaded actually are. I short cut that entire path and completed the minimum amount of work to disassociate an Entity from the PersistenceContext. http://n2.nabble.com/Extreme-performance-problems-with-OpenJPA-commit-td4497009.html
          Hide
          Pinaki Poddar added a comment -

          The recommendation to add a version field is OK
          – but what happens with this lite detachment when an entity does not use a version field?
          Can I merge() the object graph back when this detach lite was used with some entities without a version field?

          If yes, why did the test persistent entities that worked before this change without version field required addition of a version field?
          If no, how does that discrepancy manifest in the user application code – does this new mechanics detect and raise an exception when it meets an unversioned entity –
          or does such usage violate data integrity silently? Do we have a test that shows such behavior?

          Show
          Pinaki Poddar added a comment - The recommendation to add a version field is OK – but what happens with this lite detachment when an entity does not use a version field? Can I merge() the object graph back when this detach lite was used with some entities without a version field? If yes, why did the test persistent entities that worked before this change without version field required addition of a version field? If no, how does that discrepancy manifest in the user application code – does this new mechanics detect and raise an exception when it meets an unversioned entity – or does such usage violate data integrity silently? Do we have a test that shows such behavior?
          Hide
          Rick Curtis added a comment -

          > Can I merge() the object graph back when this detach lite was used with some entities without a version field?
          Yes.

          > If yes, why did the test persistent entities that worked before this change without version field required addition of a version field?
          I'm guessing that adding the @Version field is a good(/bad) habit that I've got myself into. I did some testing and I can remove the @Version field, but I would need to change one assertion in the new testcase. When there is no version field and no detached state field, the Entity doesn't know whether is is detached or not(PersistenceCapable.pcIsDetached()). The OpenJPA runtime is able to determine whether or not the Entity exists by hitting to the db looking for the Entities id. see BrokerImpl.isDetached(Object o);

          Show
          Rick Curtis added a comment - > Can I merge() the object graph back when this detach lite was used with some entities without a version field? Yes. > If yes, why did the test persistent entities that worked before this change without version field required addition of a version field? I'm guessing that adding the @Version field is a good(/bad) habit that I've got myself into. I did some testing and I can remove the @Version field, but I would need to change one assertion in the new testcase. When there is no version field and no detached state field, the Entity doesn't know whether is is detached or not(PersistenceCapable.pcIsDetached()). The OpenJPA runtime is able to determine whether or not the Entity exists by hitting to the db looking for the Entities id. see BrokerImpl.isDetached(Object o);
          Hide
          Donald Woods added a comment -

          Code checked into 2.0.0, so marking it resolved for the release notes.
          If more work is required, please open a new issue and link it to this one.

          Show
          Donald Woods added a comment - Code checked into 2.0.0, so marking it resolved for the release notes. If more work is required, please open a new issue and link it to this one.

            People

            • Assignee:
              Rick Curtis
              Reporter:
              Rick Curtis
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development