Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
RE: [eclipselink-dev] FYI: uow.discoverUnregisteredNewObjectsisinefficient and with side effect

Sebastien,

By definition private-owned means that no other objects have relationships to the ones in the relationship. I know several users have this working with shared relationships but it was not the intent.

What you are looking for, I believe, is orphan removal where the persistence provider ensures that when the last relationship to an object is broken then that object is removed. We do not yet have this support. The JPA 2.0 orphan removal (http://wiki.eclipse.org/EclipseLink/Development/JPA_2.0/orphan_removal) is basically the delete portion of private-owned.

I am unsure if we have an enhancement request for this style of orphan removal (multiple relationships). If not, we should add one as it has come up before. Let me know if this matches your requirements/expectations.

Doug

-----Original Message-----
From: Sebastien Tardif [mailto:stardif@xxxxxxxxxxxx]
Sent: Thursday, May 21, 2009 3:37 PM
To: Christopher Delahunt
Cc: eclipselink-dev@xxxxxxxxxxx
Subject: RE: [eclipselink-dev] FYI:
uow.discoverUnregisteredNewObjectsisinefficient and with side effect


Let say before this feature I removed an item from a private collection,
but forgot to set to null other(s) reference to the item. 

In my next unit of work, when I query and EclipseLink do not find the
object that the stale reference point to, it put a null in it.

So with this new feature, when I remove an item from a private
collection, and still have somewhere another reference to the item, no
delete will occur to DB.

Is it good understanding of the old and new functionality?



-----Original Message-----
From: christopher delahunt [mailto:christopher.delahunt@xxxxxxxxxx] 
Sent: Thursday, May 21, 2009 9:31 AM
To: Dev mailing list for Eclipse Persistence Services; Sebastien Tardif
Subject: Re: [eclipselink-dev] FYI:
uow.discoverUnregisteredNewObjectsisinefficient and with side effect

Hello Sebastien,

This should only affect new objects registered via a privately owned 
relationship. De-referencing the objects from the relationship will mean

they are removed from the cache (rather than being inserted anyway). The

wording on the method is correct, and removing it from the 
privateOwnedObjects only means the object is still referenced via the 
original privately owned relationship and so should be inserted. If it 
is left on the list, it is eventually unregistered.
What exactly are you experiencing, and what is the relationship to the 
objects that are expected to be deleted?

Best Regards,
Chris

Sebastien Tardif wrote:
>
> I just saw something bigger for side effect in 
> discoverUnregisteredNewObjects than setUnregisteredExistingObjects.
>
> Some of our use cases are randomly failing, and involve almost just 
> TopLink code.
>
> Other use case involving delete do not delete.
>
> New code that have nothing to do with discoverUnregisteredNewObjects 
> have been recently added to the method under revision 3676.
>
> From the JavaDoc of method removePrivateOwnedObject that seems a good 
> candidate for explaining our delete use case failing. The code seems 
> to systematically remove object from privateOwnedObjects when ever 
> it's private owned.
>
> When an object (which is referenced) is removed from the 
> privateOwnedObjects Map, it is no longer considered for removal from 
> ChangeSets and the UnitOfWork identitymap.
>
> public void iterateReferenceObjectForMapping(Object referenceObject, 
> DatabaseMapping mapping) {
>
> super.iterateReferenceObjectForMapping(referenceObject, mapping);
>
> if (mapping.isCandidateForPrivateOwnedRemoval()) {
>
> removePrivateOwnedObject(mapping, referenceObject);
>
> }
>
> }
>
> public boolean isCandidateForPrivateOwnedRemoval() {
>
> return isPrivateOwned();
>
> }
>
> public void removePrivateOwnedObject(DatabaseMapping mapping, Object 
> privateOwnedObject) {
>
> if (this.privateOwnedObjects != null) {
>
> Set objectsForMapping = this.privateOwnedObjects.get(mapping);
>
> if (objectsForMapping != null){
>
> objectsForMapping.remove(privateOwnedObject);
>
> if (objectsForMapping.isEmpty()) {
>
> this.privateOwnedObjects.remove(mapping);
>
> }
>
> }
>
> }
>
> }
>
>
------------------------------------------------------------------------
>
> *From:* eclipselink-dev-bounces@xxxxxxxxxxx 
> [mailto:eclipselink-dev-bounces@xxxxxxxxxxx] *On Behalf Of *Sebastien 
> Tardif
> *Sent:* Wednesday, May 20, 2009 10:35 AM
> *To:* Dev mailing list for Eclipse Persistence Services
> *Subject:* RE: [eclipselink-dev] FYI: 
> uow.discoverUnregisteredNewObjectsisinefficient and with side effect
>
> The method getBackkupClone() has two lines doing the cloning, maybe 
> you know for sure that these lines are never reached.
>
> - return descriptor.getObjectBuilder().buildNewInstance();
>
> - backupClone = descriptor.getObjectBuilder().buildNewInstance();
>
> I still don't like the side effect of the method 
> discoverUnregisteredNewObjects, could you provide one without side
effect?
>
> It's kind of very useful to be able to find the list of 
> knownNewObjects and unregisteredExistingObjects.
>
>
------------------------------------------------------------------------
>
> *From:* eclipselink-dev-bounces@xxxxxxxxxxx 
> [mailto:eclipselink-dev-bounces@xxxxxxxxxxx] *On Behalf Of *Gordon
Yorke
> *Sent:* Wednesday, May 20, 2009 10:26 AM
> *To:* Dev mailing list for Eclipse Persistence Services
> *Subject:* Re: [eclipselink-dev] FYI: 
> uow.discoverUnregisteredNewObjects isinefficient and with side effect
>
> getBackupClone() does not actually build anything, it just looks for 
> the backupClone. It is just a validation step and has not been added 
> but moved out of the "normal" processing stream to improve performance

> and efficiency.
>
> --Gordon
>
> Sebastien Tardif wrote:
>
> FYI: uow.discoverUnregisteredNewObjects is inefficient and with side 
> effect
>
> /**
>
> * INTERNAL:
>
> * Traverse the object to find references to objects not registered in 
> this unit of work.
>
> */
>
> *public* *void* discoverUnregisteredNewObjects(Map clones, *final* Map

> knownNewObjects, *final* Map unregisteredExistingObjects, Map 
> visitedObjects) {
>
> The JavaDoc and signature seem to convey that the only change will be 
> to populate the Maps passed as parameters.
>
> * *
>
> *The inefficiency is that in revision 2794 this line has been added:*
>
> * *
>
> getBackupClone(object, getCurrentDescriptor());
>
> We see that we ignore the return value, and we can guess that building

> a clone is not the cheaper operation.
>
> Method discoverUnregisteredNewObjects end-up modifying the UOW by 
> doing this:
setUnregisteredExistingObjects(unregisteredExistingObjects);
>
> The list of unregisteredExistingObjects will be returned anyway, so 
> that doesn't seem to be the responsibility of 
> discoverUnregisteredNewObjects to apply some logic with the result.
>
>  
>  
>  
>
>
>
------------------------------------------------------------------------
>
>
>  
>  
>  
> _______________________________________________
> eclipselink-dev mailing list
> eclipselink-dev@xxxxxxxxxxx <mailto:eclipselink-dev@xxxxxxxxxxx>
> https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
>   
>
------------------------------------------------------------------------
>
> _______________________________________________
> eclipselink-dev mailing list
> eclipselink-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
>   
_______________________________________________
eclipselink-dev mailing list
eclipselink-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev


Back to the top