Re: 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/#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} >* >* 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
--- 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} >* >* 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
--- 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} >* >* 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 > >
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} * * 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