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
> >
> >
> >