Scott,
We don't need the list at all if EM checks EMF if it's closed, then
close itself on all public APIs. But this is a bigger change and we
were wondering if you can see the list as the bottleneck (i.e. is it
worth spending time on changing the code)?
thanks,
-marina
Scott Oaks wrote:
> If there are 1000s of clients accessing the application, does the list
> have all the client EMs? Or is there a different factory for each
> client? I'm a little concerned that cycling through the list could
> become expensive if there are that many simultaneous transactions -- and
> then the list becomes its own bottleneck.
>
> I'm actually not sure about this entire thing: why can't the em.close()
> method remove itself from the list? Then the list wouldn't need to have
> WeakReferences at all, and when (if) the factory is closed, it can deal
> with the existing EMs.
>
> -Scott
>
> On Wed, 2006-08-30 at 14:28, Mitesh Meswani wrote:
>
>>Gordon Yorke wrote:
>>
>>>Hello Mitesh,
>>> This should be suffecient. We could move the code to createEntityManager instead of close(). That would ensure that the references are always cleaned up.
>>>
>>
>>Good suggestion. It can help remove the synchronized blocks guarding
>>entityManagers. However, the list "entityManagers" would keep on growing
>>till a gc kicks in and we would be iterating the growing list every time
>>we create an entitymanager. With current approach for a well behaved
>>application, the list would be trimmed for each close. So, the choice is
>>between eliminating synchronized blocks or reducing iteration size.
>>
>>Scott,
>>Any suggestions on which approach might be better? Attaching the patch
>>again for your reference
>>
>>Thanks,
>>Mitesh
>>
>>
>>
>>>--Gordon
>>>
>>>-----Original Message-----
>>>From: Mitesh.Meswani_at_Sun.COM [mailto:Mitesh.Meswani_at_Sun.COM]On Behalf Of
>>>Mitesh Meswani
>>>Sent: Tuesday, August 29, 2006 8:58 PM
>>>To: Gordon Yorke; Tom Ware
>>>Cc: persistence_at_glassfish.dev.java.net
>>>Subject: Code review for issue 1003
>>>
>>>
>>>Hi Gordon,
>>>
>>>Please find attached a fix for issue 1003
>>><https://glassfish.dev.java.net/issues/show_bug.cgi?id=1033>.
>>>Following is a summary:
>>>Changed the EntityManagerFactoryImpl.#entityManagers to be a linked list
>>>Added a new method emClosed() to EntityManagerFactoryImpl. This method
>>>is called from EntityManagerImpl.close(). The method iterates over the
>>>list and cleans up as required.
>>>
>>>Tests run:
>>>A local test exercising the code.
>>>Scott Oaks is running Specj with this code.
>>>
>>>This bug is one of the show stoppers for the UR1 release which has a HCF
>>>tomorrow (Wed). :(
>>>
>>>Please note that the fix has a limitation of not cleaning up the list in
>>>EntityManagerFactoryImpl if none of the ems of an application call
>>>close(). The implementation to solve it is a bit more involved. At this
>>>point I am wondering whether we need to maintain the list of
>>>entitymanagers at all in. I can implement the involved fix if
>>>maintaining the list is really required.
>>>
>>>Thanks,
>>>Mitesh
>>>
>>>
>>>