You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Jonathan Hurley <jh...@hortonworks.com> on 2017/01/04 16:56:45 UTC
Review Request 55178: Investigate Changing the Default Container
Policy in JPA From Vector to ArrayList
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55178/
-----------------------------------------------------------
Review request for Ambari, Nate Cole, Sebastian Toader, and Sid Wagle.
Bugs: AMBARI-19364
https://issues.apache.org/jira/browse/AMBARI-19364
Repository: ambari
Description
-------
Recently I noticed that collections coming back from JPA were using a {{Vector}} as their concrete collection. This seems very wrong as {{Vector}} is a completely synchronized collection.
Tracing through EclipseLink, it seems like this really only affects [ReadAllQuery|http://grepcode.com/file/repo1.maven.org/maven2/org.eclipse.persistence/eclipselink/2.5.2/org/eclipse/persistence/queries/ReadAllQuery.java/#68]. Essentially, EclipseLink uses a [ContainerPolicy|http://grepcode.com/file/repo1.maven.org/maven2/org.eclipse.persistence/eclipselink/2.5.2/org/eclipse/persistence/internal/queries/ContainerPolicy.java#ContainerPolicy] to determine how the result set should be collected. There are policies for {{Vector}}, {{ArrayList}}, {{HashSet}}, etc.
The interesting part here is that this really only affects the {{ReadAllQuery}} which is used by JPA's {{Query#getResultList()}} ... anytime it needs to create a collection of entities, it's going to use a {{Vector}}.
There's actually very little documentation on this. In fact, the only way to change this is either set a global value for the default container policy, or set a per-query policy. An example of setting the global policy would be:
{code}
public class EclipseLinkSessionCustomizer implements SessionCustomizer {
/**
* {@inheritDoc}
* <p/>
* This class exists for quick customization purposes.
*/
@Override
public void customize(Session session) throws Exception {
// ensure db behavior is same as shared cache
DatabaseLogin databaseLogin = (DatabaseLogin) session.getDatasourceLogin();
databaseLogin.setTransactionIsolation(DatabaseLogin.TRANSACTION_READ_COMMITTED);
// for some reason, read-all queries use a Vector as their container for
// result items - this seems like an unnecessary performance hit since
// Vectors are synchronized and there's no apparent reason to provide a
// thread-safe collection on a read all query
ContainerPolicy.setDefaultContainerClass(CoreClassConstants.ArrayList_class);
}
}
{code}
This indeed causes collections returned by queries to be backed by ArrayLists.
There is some discussion on this topic already:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=255634
It seems like this was kept as a {{Vector}} purely for backward compatibility reasons.
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/orm/EclipseLinkSessionCustomizer.java 6717e01
Diff: https://reviews.apache.org/r/55178/diff/
Testing
-------
Ran a full deployment and didn't see any issues. Verfied that ArrayList are returned in `ReadAllQuery` instead of Vector.
Thanks,
Jonathan Hurley
Re: Review Request 55178: Investigate Changing the Default Container
Policy in JPA From Vector to ArrayList
Posted by Sid Wagle <sw...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55178/#review160509
-----------------------------------------------------------
Ship it!
Ship It!
- Sid Wagle
On Jan. 4, 2017, 4:56 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55178/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2017, 4:56 p.m.)
>
>
> Review request for Ambari, Nate Cole, Sebastian Toader, and Sid Wagle.
>
>
> Bugs: AMBARI-19364
> https://issues.apache.org/jira/browse/AMBARI-19364
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Recently I noticed that collections coming back from JPA were using a {{Vector}} as their concrete collection. This seems very wrong as {{Vector}} is a completely synchronized collection.
>
> Tracing through EclipseLink, it seems like this really only affects [ReadAllQuery|http://grepcode.com/file/repo1.maven.org/maven2/org.eclipse.persistence/eclipselink/2.5.2/org/eclipse/persistence/queries/ReadAllQuery.java/#68]. Essentially, EclipseLink uses a [ContainerPolicy|http://grepcode.com/file/repo1.maven.org/maven2/org.eclipse.persistence/eclipselink/2.5.2/org/eclipse/persistence/internal/queries/ContainerPolicy.java#ContainerPolicy] to determine how the result set should be collected. There are policies for {{Vector}}, {{ArrayList}}, {{HashSet}}, etc.
>
> The interesting part here is that this really only affects the {{ReadAllQuery}} which is used by JPA's {{Query#getResultList()}} ... anytime it needs to create a collection of entities, it's going to use a {{Vector}}.
>
> There's actually very little documentation on this. In fact, the only way to change this is either set a global value for the default container policy, or set a per-query policy. An example of setting the global policy would be:
>
> {code}
> public class EclipseLinkSessionCustomizer implements SessionCustomizer {
>
> /**
> * {@inheritDoc}
> * <p/>
> * This class exists for quick customization purposes.
> */
> @Override
> public void customize(Session session) throws Exception {
> // ensure db behavior is same as shared cache
> DatabaseLogin databaseLogin = (DatabaseLogin) session.getDatasourceLogin();
> databaseLogin.setTransactionIsolation(DatabaseLogin.TRANSACTION_READ_COMMITTED);
>
> // for some reason, read-all queries use a Vector as their container for
> // result items - this seems like an unnecessary performance hit since
> // Vectors are synchronized and there's no apparent reason to provide a
> // thread-safe collection on a read all query
> ContainerPolicy.setDefaultContainerClass(CoreClassConstants.ArrayList_class);
> }
> }
> {code}
>
> This indeed causes collections returned by queries to be backed by ArrayLists.
>
> There is some discussion on this topic already:
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=255634
>
> It seems like this was kept as a {{Vector}} purely for backward compatibility reasons.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/EclipseLinkSessionCustomizer.java 6717e01
>
> Diff: https://reviews.apache.org/r/55178/diff/
>
>
> Testing
> -------
>
> Ran a full deployment and didn't see any issues. Verfied that ArrayList are returned in `ReadAllQuery` instead of Vector.
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 55178: Investigate Changing the Default Container
Policy in JPA From Vector to ArrayList
Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55178/#review160507
-----------------------------------------------------------
Ship it!
Ship It!
- Nate Cole
On Jan. 4, 2017, 11:56 a.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55178/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2017, 11:56 a.m.)
>
>
> Review request for Ambari, Nate Cole, Sebastian Toader, and Sid Wagle.
>
>
> Bugs: AMBARI-19364
> https://issues.apache.org/jira/browse/AMBARI-19364
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Recently I noticed that collections coming back from JPA were using a {{Vector}} as their concrete collection. This seems very wrong as {{Vector}} is a completely synchronized collection.
>
> Tracing through EclipseLink, it seems like this really only affects [ReadAllQuery|http://grepcode.com/file/repo1.maven.org/maven2/org.eclipse.persistence/eclipselink/2.5.2/org/eclipse/persistence/queries/ReadAllQuery.java/#68]. Essentially, EclipseLink uses a [ContainerPolicy|http://grepcode.com/file/repo1.maven.org/maven2/org.eclipse.persistence/eclipselink/2.5.2/org/eclipse/persistence/internal/queries/ContainerPolicy.java#ContainerPolicy] to determine how the result set should be collected. There are policies for {{Vector}}, {{ArrayList}}, {{HashSet}}, etc.
>
> The interesting part here is that this really only affects the {{ReadAllQuery}} which is used by JPA's {{Query#getResultList()}} ... anytime it needs to create a collection of entities, it's going to use a {{Vector}}.
>
> There's actually very little documentation on this. In fact, the only way to change this is either set a global value for the default container policy, or set a per-query policy. An example of setting the global policy would be:
>
> {code}
> public class EclipseLinkSessionCustomizer implements SessionCustomizer {
>
> /**
> * {@inheritDoc}
> * <p/>
> * This class exists for quick customization purposes.
> */
> @Override
> public void customize(Session session) throws Exception {
> // ensure db behavior is same as shared cache
> DatabaseLogin databaseLogin = (DatabaseLogin) session.getDatasourceLogin();
> databaseLogin.setTransactionIsolation(DatabaseLogin.TRANSACTION_READ_COMMITTED);
>
> // for some reason, read-all queries use a Vector as their container for
> // result items - this seems like an unnecessary performance hit since
> // Vectors are synchronized and there's no apparent reason to provide a
> // thread-safe collection on a read all query
> ContainerPolicy.setDefaultContainerClass(CoreClassConstants.ArrayList_class);
> }
> }
> {code}
>
> This indeed causes collections returned by queries to be backed by ArrayLists.
>
> There is some discussion on this topic already:
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=255634
>
> It seems like this was kept as a {{Vector}} purely for backward compatibility reasons.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/EclipseLinkSessionCustomizer.java 6717e01
>
> Diff: https://reviews.apache.org/r/55178/diff/
>
>
> Testing
> -------
>
> Ran a full deployment and didn't see any issues. Verfied that ArrayList are returned in `ReadAllQuery` instead of Vector.
>
>
> Thanks,
>
> Jonathan Hurley
>
>
Re: Review Request 55178: Investigate Changing the Default Container
Policy in JPA From Vector to ArrayList
Posted by Sebastian Toader <st...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55178/#review160525
-----------------------------------------------------------
Ship it!
Ship It!
- Sebastian Toader
On Jan. 4, 2017, 5:56 p.m., Jonathan Hurley wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55178/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2017, 5:56 p.m.)
>
>
> Review request for Ambari, Nate Cole, Sebastian Toader, and Sid Wagle.
>
>
> Bugs: AMBARI-19364
> https://issues.apache.org/jira/browse/AMBARI-19364
>
>
> Repository: ambari
>
>
> Description
> -------
>
> Recently I noticed that collections coming back from JPA were using a {{Vector}} as their concrete collection. This seems very wrong as {{Vector}} is a completely synchronized collection.
>
> Tracing through EclipseLink, it seems like this really only affects [ReadAllQuery|http://grepcode.com/file/repo1.maven.org/maven2/org.eclipse.persistence/eclipselink/2.5.2/org/eclipse/persistence/queries/ReadAllQuery.java/#68]. Essentially, EclipseLink uses a [ContainerPolicy|http://grepcode.com/file/repo1.maven.org/maven2/org.eclipse.persistence/eclipselink/2.5.2/org/eclipse/persistence/internal/queries/ContainerPolicy.java#ContainerPolicy] to determine how the result set should be collected. There are policies for {{Vector}}, {{ArrayList}}, {{HashSet}}, etc.
>
> The interesting part here is that this really only affects the {{ReadAllQuery}} which is used by JPA's {{Query#getResultList()}} ... anytime it needs to create a collection of entities, it's going to use a {{Vector}}.
>
> There's actually very little documentation on this. In fact, the only way to change this is either set a global value for the default container policy, or set a per-query policy. An example of setting the global policy would be:
>
> {code}
> public class EclipseLinkSessionCustomizer implements SessionCustomizer {
>
> /**
> * {@inheritDoc}
> * <p/>
> * This class exists for quick customization purposes.
> */
> @Override
> public void customize(Session session) throws Exception {
> // ensure db behavior is same as shared cache
> DatabaseLogin databaseLogin = (DatabaseLogin) session.getDatasourceLogin();
> databaseLogin.setTransactionIsolation(DatabaseLogin.TRANSACTION_READ_COMMITTED);
>
> // for some reason, read-all queries use a Vector as their container for
> // result items - this seems like an unnecessary performance hit since
> // Vectors are synchronized and there's no apparent reason to provide a
> // thread-safe collection on a read all query
> ContainerPolicy.setDefaultContainerClass(CoreClassConstants.ArrayList_class);
> }
> }
> {code}
>
> This indeed causes collections returned by queries to be backed by ArrayLists.
>
> There is some discussion on this topic already:
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=255634
>
> It seems like this was kept as a {{Vector}} purely for backward compatibility reasons.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/orm/EclipseLinkSessionCustomizer.java 6717e01
>
> Diff: https://reviews.apache.org/r/55178/diff/
>
>
> Testing
> -------
>
> Ran a full deployment and didn't see any issues. Verfied that ArrayList are returned in `ReadAllQuery` instead of Vector.
>
>
> Thanks,
>
> Jonathan Hurley
>
>